Uploaded image for project: 'Couchbase Python Client Library'
  1. Couchbase Python Client Library
  2. PYCBC-749

Remove additional overloads for query and search

    XMLWordPrintable

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 3.0.0-rc
    • None
    • None
    • 1

    Description

      If I'm getting something wrong that python needs I'm sorry. Looking at the Cluster API I can see that I think analytics has (properly?) only one overload:

       def analytics_query(self,  # type: Cluster
                              statement,  # type: str,
                              *options,  # type: AnalyticsOptions
                              **kwargs
                              ):
      

      query has also these two which I think are not necessary:

          @overload
          def query(self,
                    statement,
                    parameters=None,
                    timeout=None):
              pass
       
          @overload
          def query(self,
                    statement,  # type: str,
                    *options  # type: QueryOptions
                    ):
              # type: (...) -> IQueryResult
              pass
      

      same for search

          @overload
          def search_query(self,
                           index,  # type: str
                           query,  # type: Union[str, Query]
                           facets=None  # type: Mapping[str,Facet]
                           ):
              pass
       
          @overload
          def search_query(self,
                           index,  # type: str
                           query,  # type: Union[str, Query]
                           options,  # type: SearchOptions
                           ):
              pass
      

      For query i.e. two versions should be possible: query("statement") and query("statement", options)

      Note: I understand that there is still some discussion going on how how the overloading should be done (options vs. named params) but whatever the decision is going to be the APIs should be in sync.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          Ellis.Breen Ellis Breen added a comment -

          Agreed these should be rejigged, to support QueryOptions properly, as PYCBC-694.

          Overloads not necessary as per RFC, just a nice-to-have for e.g. named parameters. Positional parameters slightly ambiguous with the presence of QueryOptions, but should be possilbe to disambiguate-able as these would be `str`s vs a QueryOptions block.

          Ellis.Breen Ellis Breen added a comment - Agreed these should be rejigged, to support QueryOptions properly, as PYCBC-694 . Overloads not necessary as per RFC, just a nice-to-have for e.g. named parameters. Positional parameters slightly ambiguous with the presence of QueryOptions, but should be possilbe to disambiguate-able as these would be `str`s vs a QueryOptions block.
          david.kelly David Kelly added a comment -

          Fixed in PYCBC-694

          david.kelly David Kelly added a comment - Fixed in PYCBC-694

          People

            david.kelly David Kelly
            daschl Michael Nitschinger
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty