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

Delete takes care of unscheduling #23

Merged
merged 4 commits into from
Jul 10, 2018

Conversation

tom-price
Copy link
Collaborator

A revised version of PR #20 simplified to only include revisions to how deletion works (no admin validation on time) that'll be safe to merge after PR #22.

@tom-price tom-price closed this Jul 1, 2018
@tom-price
Copy link
Collaborator Author

tom-price commented Jul 1, 2018

Wasn't sure the best way to submit this PR as its tests fail without the revisions present in #22 so I cherry-picked the two revisions in that branch that are relevant.

@tom-price tom-price reopened this Jul 1, 2018
tom-price and others added 4 commits July 1, 2018 00:11
And SessionAuthenticationMiddleware class is removed in 2.0 as it
provided no functionality since session authentication is unconditionally
enabled in Django 1.10.
In python2.x d.keys() returned a list. With python3.x, d.keys() returns a dict_keys object which behaves a lot more like a set than a list. As such, it can't be indexed.

Changing d.keys() to list(d)) will return a list of keys on both python2.x and python3.x without making any copies.

reference: https://stackoverflow.com/questions/17322668/typeerror-dict-keys-object-does-not-support-indexing
delete takes care of unscheduling
admin uses model.delete instead of model.all.delete
tests updated
@tom-price tom-price force-pushed the unschedule-delete_after_ttl branch from 4c3b3e3 to 12f6a5d Compare July 1, 2018 04:20
@bashhack bashhack merged commit fa76c6a into islco:master Jul 10, 2018
@oudeismetis
Copy link
Contributor

@tom-price Some great work!
Would you like to be added as a maintainer for this project?

After we update the README and get a new pypi version out there, @bashhack and I are going to continue to keep an eye on this project for issues and PRs, but it's not going to be a high priority for us.

I think the 3 of us together could easily make sure this project stays relevant and up to date with it's current features.

@tom-price
Copy link
Collaborator Author

tom-price commented Jul 11, 2018

Sure, I'd be happy to help. I'm using this as part of a longer term project of my own so I'll also be paying attention to it, rq-scheduler, etc..

As a heads up, I've been working on adding args and kwargs support and will hopefully finish sometime later this week. A real blocker has been adding tests that assert the args and kwargs are getting properly passed all the way through.

@jteppinette
Copy link

@oudeismetis @tom-price Is django-rq-scheduler going to be updated on PyPI?

@tom-price
Copy link
Collaborator Author

@jteppinette, Unfortunately I'm not sure. I've not been in touch with the other maintainers and I've not done that part before. I've got a couple changes ready to go and continue to use a fork of this project on a daily basis in my own work, so it's not forgotten about.

@jteppinette
Copy link

@tom-price It looks like someone is maintaining a separate PyPI package with this update and .. something else .. (1.1.3).

https://pypi.org/project/readwise-django-rq-scheduler/#description

@tom-price
Copy link
Collaborator Author

Interesting. I'll make a renewed effort to get in touch with the other maintainers. There are some outstanding PRs including my own, #24, that seem ready, or would be with little work, for merging in and publishing.

@jteppinette
Copy link

@oudeismetis @tom-price We should update PyPI ownership on this package. I am using this package at work, so I would be willing to do it.

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.

5 participants