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

get_fulldoc and upsert_fulldoc should not be in subdoc API

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.2.6
    • 2.3.0
    • library
    • None

    Description

      The current Python client implementation has some extra stuff that I think maybe leftover from earlier experiments and should be removed so we don't cause confusion about conformity.

      Found when reviewing the sdk-rfc:

      cb.lookup_in(docid, SD.get_fulldoc(), SD.get('my.xattr', xattr=True)) 
      cb.mutate_in(docid, SD.upsert_fulldoc(docid), SD.upsert('my.xattr', xattr=True)) 
      

      Looking around, I don't see any indication that the get_fulldoc or upsert_fulldoc exist elsewhere.

      Will confirm and then remove if needed.

       

      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 -

          Suggested change pushed.

          Ellis.Breen Ellis Breen added a comment - Suggested change pushed.
          Ellis.Breen Ellis Breen added a comment -

          Not sure where those code snippets are from, but the only references I can find to upsert/get_fulldoc are in priv_constants.py and subdoc_t.py . It is mentioned here:

          "PYCBC-404, PYCBC-402: GET_COUNT and other subdoc additions

           

          This adds the `get_count()` subdocument operation. It also adds support

          for the `get_fulldoc()` and `upsert_fulldoc()` operations which can be

          used to retrieve and modify the entire document, respectively."

          According to the sdk-rfc mentioned in PCYCB-404 ( https://docs.google.com/document/d/1z3pJCPg77PZ8U8rFAyABuEhHSl68j1n42zP2RyflWZs/edit#heading=h.7ha864pzwf8m ):

          "In the SDKs the proposed way to expose this is by allowing an empty path to the get() and upsert() sub-functions of LookupInBuilder. MutateInBuilder." - as Brett Lawson reiterated.

          Seems pretty safe to remove as far as I can tell - will try tomorrow.

           

          Ellis.Breen Ellis Breen added a comment - Not sure where those code snippets are from, but the only references I can find to upsert/get_fulldoc are in priv_constants.py and subdoc_t.py . It is mentioned here: " PYCBC-404 , PYCBC-402 : GET_COUNT and other subdoc additions   This adds the `get_count()` subdocument operation. It also adds support for the `get_fulldoc()` and `upsert_fulldoc()` operations which can be used to retrieve and modify the entire document, respectively." According to the sdk-rfc mentioned in PCYCB-404 (  https://docs.google.com/document/d/1z3pJCPg77PZ8U8rFAyABuEhHSl68j1n42zP2RyflWZs/edit#heading=h.7ha864pzwf8m  ): "In the SDKs the proposed way to expose this is by allowing an empty path to the get() and upsert() sub-functions of LookupInBuilder . MutateInBuilder ." - as Brett Lawson reiterated. Seems pretty safe to remove as far as I can tell - will try tomorrow.  
          brett19 Brett Lawson added a comment -

          Hey Matt,

          According to the RFC, if your SDK implements fulldoc operations (for use in concert with xattrs), then you would implement it by using a blank path as opposed to an independent method.

          Cheers, Brett

          brett19 Brett Lawson added a comment - Hey Matt, According to the RFC, if your SDK implements fulldoc operations (for use in concert with xattrs), then you would implement it by using a blank path as opposed to an independent method. Cheers, Brett

          Brett Lawson: as sdk-rfc owner, can you confirm?  I didn't see this in go or the sdk-rfc.  You can assign it back to me or to Michael Goldsmith after confirming.

          ingenthr Matt Ingenthron added a comment - Brett Lawson : as sdk-rfc owner, can you confirm?  I didn't see this in go or the sdk-rfc.  You can assign it back to me or to Michael Goldsmith after confirming.

          People

            Ellis.Breen Ellis Breen
            ingenthr Matt Ingenthron
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              PagerDuty