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

New Cache: OrderedCache #23

Merged
merged 4 commits into from
Jul 28, 2016
Merged

New Cache: OrderedCache #23

merged 4 commits into from
Jul 28, 2016

Conversation

rkubik
Copy link
Contributor

@rkubik rkubik commented Jul 26, 2016

I've found the need to query the cache based on order. If there is interest I can add more features.

@grantjenks
Copy link
Owner

Looks good. Can you describe your use case?

I've been interested in something similar. Though I wanted to define __iter__ and __reversed__. I think that would get you your first and last like: next(iter(cache)) and next(reversed(cache)). Would those fit your use case? I was also thinking of only yielding keys.

@rkubik
Copy link
Contributor Author

rkubik commented Jul 27, 2016

Can you describe your use case?

A cache for managing TCP requests when the app is offline (no network connection). All requests during this time consult the cache which should only store unique requests (the app may duplicate requests based on user input). Once network connection is restored the app will process the TCP requests in order to prevent starvation.

I've been interested in something similar. Though I wanted to define iter and reversed.

That sounds like a better, more Pythonic approach.

I was also thinking of only yielding keys.

That works.

Do you have the time to implement this if you have a design in mind? I don't mind doing it, though.

@grantjenks
Copy link
Owner

If you have time, please do and I'll merge/deploy.

The design should be pretty simple. I would use a generator and track the rowid.

@rkubik
Copy link
Contributor Author

rkubik commented Jul 27, 2016

@grantjenks Please take a look. Also, not sure why the pypy build is failing.

@grantjenks
Copy link
Owner

If you rerun the PyPy test it will work. There's a bug open for the scheme change error.

@grantjenks
Copy link
Owner

You've also got to call disk.get to convert the database key to a python object.

Also, can you do lookups in chunks rather than fetchall? Use the cull_limit for chunk size.

@rkubik
Copy link
Contributor Author

rkubik commented Jul 27, 2016

Thanks for the feedback, please take a look again.

@grantjenks
Copy link
Owner

Looks good. Can you put that in a while loop to iterate all keys now?

I'm a little concerned that iter may never end. Reversed will end at rowid 0 but if keys are added continuously then iter will keep going. You could reload the "count" attribute and use that to limit iteration. What do you think?

@rkubik
Copy link
Contributor Author

rkubik commented Jul 27, 2016

I'm a little concerned that iter may never end.

Honestly, I don't think that is a problem. Doing our own magic to stop the iteration may be more convoluted than letting it continue on.

You could reload the "count" attribute and use that to limit iteration. What do you think?

We could store the rowid and change the query to:

# pseudo code __iter__
def __iter__(self):
    self._rowid_offset = 0
    while True:
        rows = sql('SELECT rowid, key, raw FROM Cache WHERE rowid >= ? ORDER BY rowid ASC LIMIT ?', (self._rowid_offset, chunk))
        if not rows:
           break
        self._rowid_offset = rows[len(rows)-1][0] + 1
        for (_, key, raw) in rows:
            yield ....

Same concept for reversed. Thoughts?

@grantjenks grantjenks merged commit 4cc9d89 into grantjenks:master Jul 28, 2016
@rkubik
Copy link
Contributor Author

rkubik commented Jul 28, 2016

Did you mean to merge that?

@grantjenks
Copy link
Owner

Yep. I'll fix it up. I'm planning a new release tonight.

@grantjenks
Copy link
Owner

Deployed at v1.7.0 to PyPI.

I included only the iter and reversed addition to core.Cache. Use those to implement your first/last behavior.

Thank you for your contribution.

@rkubik
Copy link
Contributor Author

rkubik commented Jul 28, 2016

Awesome, thank you.

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

Successfully merging this pull request may close these issues.

None yet

2 participants