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

Allow passing backend classes from CLI and other APIs #786

Merged
merged 2 commits into from Feb 3, 2017
Merged

Allow passing backend classes from CLI and other APIs #786

merged 2 commits into from Feb 3, 2017

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented Jan 23, 2017

This is related to rq/Flask-RQ2#2 and rq/Flask-RQ2#12. While debugging those issues I've found many inconsistencies how the backend classes are used throughout the RQ code base, basically making it currently impossible to fully override job, queue, worker or connection classes.

This should be backward-compatible, but any testing in real-world projects would be appreciated.

@coveralls
Copy link

coveralls commented Jan 23, 2017

Coverage Status

Coverage increased (+1.5%) to 87.551% when pulling a50c257 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 87.551% when pulling 3479765 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 87.551% when pulling 3479765 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 87.551% when pulling 3479765 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 87.551% when pulling 3479765 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

@coveralls
Copy link

coveralls commented Jan 24, 2017

Coverage Status

Coverage increased (+1.6%) to 87.61% when pulling 81e0ab5 on jezdez:backend-class-overrides into 4fc032b on nvie:master.

@@ -626,7 +626,7 @@ class TimeoutTestCase:
def setUp(self):
# we want tests to fail if signal are ignored and the work remain
# running, so set a signal to kill them after X seconds
self.killtimeout = 10
self.killtimeout = 15
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI this was the only way I was able to fix a slow subprocess call in the pypy env on Travis CI.

@@ -123,7 +124,7 @@ def create(cls, func, args=None, kwargs=None, connection=None,
job._instance = func
job._func_name = '__call__'
else:
raise TypeError('Expected a callable or a string, but got: {}'.format(func))
raise TypeError('Expected a callable or a string, but got: {0}'.format(func))
Copy link
Member Author

Choose a reason for hiding this comment

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

Change needed to work on Python 2.6.

@selwin
Copy link
Collaborator

selwin commented Jan 24, 2017

Wow, this is a big PR. I'll need some time to review this. If possible, next time please split them into smaller, logically grouped PRs :)

@jezdez
Copy link
Member Author

jezdez commented Jan 25, 2017

@selwin I'm sorry for the size but I don't see how I could have cut the PR for passing the backend classes into smaller chunks, it just was in a lot of places missing. I'd be happy to update the patch if there is a better way to handle passing around those classes of course.

…LI and other APIs

This includes:

- a partial refactor of the CLI to organize the shared options
- extends the tests in areas where passing custom backend classes makes sense
- allow setting the core CLI options as env vars
- minor cosmetic changes here and there
@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 87.748% when pulling c019662 on jezdez:backend-class-overrides into b241d50 on nvie:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 87.748% when pulling c019662 on jezdez:backend-class-overrides into b241d50 on nvie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 87.748% when pulling c019662 on jezdez:backend-class-overrides into b241d50 on nvie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 87.748% when pulling c019662 on jezdez:backend-class-overrides into b241d50 on nvie:master.

@selwin
Copy link
Collaborator

selwin commented Feb 3, 2017

Thanks for the PR @jezdez !

@selwin selwin merged commit 83007b2 into rq:master Feb 3, 2017
@jezdez jezdez deleted the backend-class-overrides branch February 3, 2017 10:55
@jezdez
Copy link
Member Author

jezdez commented Feb 3, 2017

@selwin Happy to :)

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

3 participants