limeuser / appengine-ndb-experiment

Automatically exported from code.google.com/p/appengine-ndb-experiment
Other
0 stars 0 forks source link

Add a _post_create_hook() to the Model class. #214

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The behaviour of a '_post_create_hook' can be achieved by changing the models 
constructor and also by hacking around with the '_post_put_hook'.
(The latter being the better choice in this case.)

Yet it would be cleaner and more natural, to have a new hook that would act 
like the '_post_put_hook', but only be called on the first put() for an entity.
One would not need rather ugly hacks to bend the constructor (which is called 
rather often) or the '_post_put_hook' into the desired behaviour.

The hook would be the logical counterpart to the delete hooks.

Original issue reported on code.google.com by M.Zettlm...@gmail.com on 5 Oct 2012 at 11:13

GoogleCodeExporter commented 9 years ago
I don't think the effects of hacking the constructor and the post-put hook 
could be at all similar, given that the former gets called before the item is 
written to the datastore and the latter, after.

Assuming you really want a hook that is called after a successful put() but 
only if the instance did not exist before, that's still pretty tricky: the 
datastore doesn't actually return any information to distinguish an insert from 
an update.  If you are willing to let the datastore assign numeric instance 
IDs, you could do it by combining a pre-put hook (which records whether there 
is a valid key as some flag on the instance) and a post-put hook (which checks 
for the flag).

You could also possibly use the known hacks for approximating the old db's 
is_saved flag (check stackoverflow).

I'd like to see more stars on this feature request before I believe it's 
needed.  Specific stories about how you plan to use this would be helpful too.

Original comment by guido@google.com on 5 Oct 2012 at 11:28

GoogleCodeExporter commented 9 years ago
Right now I am doing it with '_pre_put_hook' and '_post_put_hook', checking for 
the existence of an entities id and setting an instance variable. A class 
variable serves as default value.

Put a bit simplified, I'm incrementing and decrementing a reference counter on 
another entity. (Can't count the references directly though.)
I can imagine more use-cases and also, that other people would probably show 
interest in such a hook.

I also took a look at your post on stackoverflow. It is rather similar to what 
I am currently doing.
Although your '_post_get_hook'-approach obviously takes one less call in the 
case that the entity did not previously exist.

Your point with wanting to see more interest in this feature is understandable, 
especially if the datastore returns no distinguishing information.
And as long as the datastore keeps operating in the same way, everything you 
could do, would essentially be the same thing that I'm already doing, just on 
another level. An increased readability and ease of use would be the benefit of 
a '_post_create_hook'.

Original comment by M.Zettlm...@gmail.com on 6 Oct 2012 at 12:36

GoogleCodeExporter commented 9 years ago
Actually the '_post_get_hook'-solution is superior to the 
'_pre_put_hook'-solution in terms of general applicability. It also works with 
user-assigned ids due to the different location of the check in the process of 
loading and saving the entity.

Would you generally be willing to accept a '_post_create_hook' in the module 
regardless of interest? (e.g. if supplied a patch)
Or only after a certain interest-threshold is reached, to not bloat the program 
with minor features?

Original comment by M.Zettlm...@gmail.com on 6 Oct 2012 at 1:11

GoogleCodeExporter commented 9 years ago
At this point I think it makes more sense for a group of advanced users to try 
and come up with a set of best practices rather than requesting more features 
for the core library.  About the specific idea of a post-create hook: if it 
won't work right when the user assigns the key, it's too much of a liability to 
have it in the library.

Original comment by guido@google.com on 6 Oct 2012 at 1:35

GoogleCodeExporter commented 9 years ago
In the light of a new day, I can clearly see how it would be a liability. It 
probably was too late at night and I should not have added that last part.
It would be quite a liability if it wouldn't work right upon key assignment, 
because obviously one does not have to get() an entity, before creating an 
entity with the same key as an existing one. I had not thought of that case 
last night.

Basically I can not see a "quick enhancement". (But my insight into the library 
is not as big as yours, since I only went over the code once.)
Since the datastore currently works the way it does, every attempt to solve 
this in the current version of the library would just be rather hacked.
My way always worked very well for me and I just thought that it would be a 
good enhancement to the library to have such a hook.

Now, after having seen your points and having put thought into them, I still 
think that the hook in general would be good, but only if it some day comes 
along naturally, due to general changes in the behaviour of the datastore.

It was good to create this feature request, since this way the hurdle for 
people to express their interest in such a feature is lowered. And if enough 
people show their interest over time, it might one day be justifiable to change 
the behaviour on a lower level.

Original comment by M.Zettlm...@gmail.com on 6 Oct 2012 at 11:23