Skip to content
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

MetaGetter* Get functions should be changed to take (aid, ns) #41

Open
riannucci opened this issue Mar 16, 2016 · 3 comments
Open

MetaGetter* Get functions should be changed to take (aid, ns) #41

riannucci opened this issue Mar 16, 2016 · 3 comments

Comments

@riannucci
Copy link
Member

Currently gae has 'special' metadata fields which can be of type *Key. However, there's no way to synthesize a *Key correctly without also having the AppID and Namespace strings. Either you can use the datatstore.Interface.MakeKey method (which takes them from the context), or you can use the datastore.MakeKey, which requires them to be passed in.

I would prefer to pass these strings in the Get* functions, rather than passing the context. If we pass the context then there's a risk that MetaGetter implementations could do naughty things like talk to the datastore, or potentially even deadlock (by calling other MetaGetter types, which are non-obviously mutually recursive). Passing the (aid, ns) context explicitly doesn't have this risk, and it makes it obvious what this contextual information should be used for.

@riannucci
Copy link
Member Author

@nodirt

@riannucci
Copy link
Member Author

@danjacques do your recent key changes help this issue out?

@danjacques
Copy link
Contributor

Yes. Introduction of KeyContext gives us a key generation struct that can do all of that without access to Context. You can generate a KeyContext from a Context or manually via MkKeyContext.

The MetaGetterSetter interface doesn't know how to take advantage of KeyContext though; however, MGS also doesn't take a Context argument, so I'm not sure what the latter half of the first comment is referring to.

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

No branches or pull requests

2 participants