New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Somehow warn when passing the `_key` attribute to `initValues` #118

Closed
larsborn opened this Issue Sep 22, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@larsborn

larsborn commented Sep 22, 2018

I just started to use this Python client for ArangoDB and - stupidly - stumbled into a pitfall. I wanted to set a custom key for my documents and set it via _key of the dictionary passed to initValues of createDocument. I understand that ArangoDB client should ignore all attributed starting with an underscore (because they are supposed to be used by Arango internally) so the behavior of ignoring the value I passed is probably intentional.

My solution was, to change this

json_data = {'foo': 'bar'}
json_data['_key'] = 'your_key'
doc = connection['your_database']['your_collection'].createDocument(initValues=json_data)
doc.save()

to this

json_data = {'foo': 'bar'}
doc = connection['your_database']['your_collection'].createDocument(initValues=json_data)
doc._key = 'your_key'
doc.save()

I would greatly have appreciated a warning in the first code snippet above though. Another option would of course be to make an exception for the dictionary entry _key and just set the value in the document. _key is somehow different from other internal properties since it occasionally is set manually.

What do you think?

@danielkiedrowski

This comment has been minimized.

Show comment
Hide comment
@danielkiedrowski

danielkiedrowski Sep 22, 2018

Contributor

Hey @larsborn! I actually do exactly what you did in your first snippet and it works as expected. I can set a custom _key with the init dict and nothing is getting ignored for me.

Looking into the code of pyArango, I don't see any ignoring happening there. At some point during the initialization of you Documentthe following code will be called from reset(self, collection, jsonFieldInit = {}):

    def setPrivates(self, fieldDict) :
        """will set self._id, self._rev and self._key field."""
        
        for priv in self.privates :
            if priv in fieldDict :
                setattr(self, priv, fieldDict[priv])
            else :
                setattr(self, priv, None)
        
        if self._id is not None :
            self.URL = "%s/%s" % (self.documentsURL, self._id)

Have you stepped through the createDocument process and inspected when your _keygets lost?

Contributor

danielkiedrowski commented Sep 22, 2018

Hey @larsborn! I actually do exactly what you did in your first snippet and it works as expected. I can set a custom _key with the init dict and nothing is getting ignored for me.

Looking into the code of pyArango, I don't see any ignoring happening there. At some point during the initialization of you Documentthe following code will be called from reset(self, collection, jsonFieldInit = {}):

    def setPrivates(self, fieldDict) :
        """will set self._id, self._rev and self._key field."""
        
        for priv in self.privates :
            if priv in fieldDict :
                setattr(self, priv, fieldDict[priv])
            else :
                setattr(self, priv, None)
        
        if self._id is not None :
            self.URL = "%s/%s" % (self.documentsURL, self._id)

Have you stepped through the createDocument process and inspected when your _keygets lost?

@larsborn

This comment has been minimized.

Show comment
Hide comment
@larsborn

larsborn Sep 22, 2018

Hi @danielkiedrowski thanks for your quick reply!

After confirming the bug in my environment, I did some digging through the code myself. It turns out, the most recent version installed through pip (which is 1.3.1, see https://pypi.org/project/pyArango/) contains a different implementation of setPrivates:

def setPrivates(self, fieldDict) :
    """will set self._id, self._rev and self._key field."""

    try :
        for priv in ["_id", "_key", "_rev"] :
            setattr(self, priv, fieldDict[priv])
        self.URL = "%s/%s" % (self.documentsURL, self._id)
    except KeyError :
        for priv in ["_id", "_key", "_rev"] :
            setattr(self, priv, None)
        self.URL = None

And if, for example, only _key is set (and not _id) it will execute the except branch and set all private attributes to None. Probably the reason why it was changed in the first place ;).

Sorry for not stating details on my environment, this would probably have saved us both time. Any chance to get release of the 1.3.2 version on pypi?

larsborn commented Sep 22, 2018

Hi @danielkiedrowski thanks for your quick reply!

After confirming the bug in my environment, I did some digging through the code myself. It turns out, the most recent version installed through pip (which is 1.3.1, see https://pypi.org/project/pyArango/) contains a different implementation of setPrivates:

def setPrivates(self, fieldDict) :
    """will set self._id, self._rev and self._key field."""

    try :
        for priv in ["_id", "_key", "_rev"] :
            setattr(self, priv, fieldDict[priv])
        self.URL = "%s/%s" % (self.documentsURL, self._id)
    except KeyError :
        for priv in ["_id", "_key", "_rev"] :
            setattr(self, priv, None)
        self.URL = None

And if, for example, only _key is set (and not _id) it will execute the except branch and set all private attributes to None. Probably the reason why it was changed in the first place ;).

Sorry for not stating details on my environment, this would probably have saved us both time. Any chance to get release of the 1.3.2 version on pypi?

@danielkiedrowski

This comment has been minimized.

Show comment
Hide comment
@danielkiedrowski
Contributor

danielkiedrowski commented Sep 23, 2018

Exactly! @tariqdaouda
#105

@tariqdaouda

This comment has been minimized.

Show comment
Hide comment
@tariqdaouda

tariqdaouda Sep 26, 2018

Owner

My PhD defence is over. I am planning on releasing the current dev version tonight on pip!

Owner

tariqdaouda commented Sep 26, 2018

My PhD defence is over. I am planning on releasing the current dev version tonight on pip!

@tariqdaouda

This comment has been minimized.

Show comment
Hide comment
@tariqdaouda

tariqdaouda Sep 27, 2018

Owner

I am closing this as 1.3.2 Has just been released on pypi and github!

Owner

tariqdaouda commented Sep 27, 2018

I am closing this as 1.3.2 Has just been released on pypi and github!

@larsborn

This comment has been minimized.

Show comment
Hide comment
@larsborn

larsborn Sep 27, 2018

@tariqdaouda congratulations man! I hope everything went well and that you are happy with the results!! :)

larsborn commented Sep 27, 2018

@tariqdaouda congratulations man! I hope everything went well and that you are happy with the results!! :)

@tariqdaouda

This comment has been minimized.

Show comment
Hide comment
@tariqdaouda

tariqdaouda Sep 28, 2018

Owner

Thanks @larsborn! Everything went very well :)

Owner

tariqdaouda commented Sep 28, 2018

Thanks @larsborn! Everything went very well :)

@larsborn

This comment has been minimized.

Show comment
Hide comment
@larsborn

larsborn Sep 29, 2018

@tariqdaouda releasing 1.3.2. should have fixed #119, #113 and #111 too.

larsborn commented Sep 29, 2018

@tariqdaouda releasing 1.3.2. should have fixed #119, #113 and #111 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment