Skip to content
This repository has been archived by the owner on Feb 7, 2019. It is now read-only.

Only use query_time when it has explicitly been set via as_of #17

Merged

Conversation

brki
Copy link
Contributor

@brki brki commented Oct 17, 2014

I propose that we change how query_time is handled.

Up until now, code like:

# Authors.books is a many-to-many relationship
Authors.objects.create(name="Joe")
a1 = Authors.objects.current(name="Joe")
book = Books.objects.create(name="The love life of beetles")
a1.books.add(book)
print a1.books.all().count()

would print out "0", because when a1 was fetched, it's query_time was set to the time the fetch query was made. I think it should print out "1".

What I'm proposing, is that an object that is fetched via objects.current, or has had it's as_of time explicitly set to None, will reflect the ongoing current state, not the state at the time when the object was fetched.

I propose this for two reasons:

  1. It makes more sense to me.
  2. It helps another pull request I'll make work. The other pull request deals with adding multiple objects at once to a many-to-many relationship (e.g. a1.books.add(book1, book2, book3)).

& Q(version_start_date__lte=querytime)
else:
filter = Q(version_end_date__isnull=True)
return queryset.filter(filter)

def set_as_of(self, time=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can be removed since not used.

@maennel
Copy link
Contributor

maennel commented Oct 20, 2014

Thanks @brki for the PR! There are some open questions, but nothing blocking!

maennel added a commit that referenced this pull request Oct 20, 2014
Only use query_time when it has explicitly been set via as_of
@maennel maennel merged commit 9d872b3 into swisscom:master Oct 20, 2014
@maennel
Copy link
Contributor

maennel commented Oct 20, 2014

@brki Please see the cleanup_querytime_limit branch where I've adapted the code as described.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants