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

More usuable stuff #1

Merged
merged 29 commits into from Nov 12, 2012
Merged

More usuable stuff #1

merged 29 commits into from Nov 12, 2012

Conversation

jeanphix
Copy link
Contributor

  • removes the circular dependency between app and RQ instance
  • helpers are now module to level member
  • adds DSN configuration for queues
  • doc reflect changes
  • tests
  • travisci integration

@mattupstate
Copy link
Collaborator

Wow. I'm rather busy today but I can't wait to review all this.
On Oct 26, 2012 3:17 AM, "jean-philippe serafin" notifications@github.com
wrote:

  • removes the circular dependency between app and RQ instance
  • helpers are now module to level member
  • adds DSN configuration for queues
  • doc reflect changes
  • tests
  • travisci integration

You can merge this Pull Request by running:

git pull https://github.com/Birdback/flask-rq master

Or view, comment on, or merge it at:

#1
Commit Summary

  • Improves gitignore that now ignores vim & autoenv files
  • Moves function module level
  • Cleans up RQ class that no more needs circular dependencies
  • Removes useless get_app() function
  • Fixes default kwargs
  • Adds config_value tests
  • Adds get_connection() test for defaults
  • Fixes get_queue() now returns queue with appropriate connection
  • Typos
  • Removes enqueue() that pop a name kwargs that should be sendable
    to…
  • Improves @task https://github.com/task decorator that now use rq
    builtin @job https://github.com/job decorator
  • Adds configs now use defaults if not set
  • Connections can now be set by DSN
  • Adds a get_worker() function
  • Pep8
  • Adds .travis.yml
  • Adds build status to README
  • Improves docs
  • Fixes README header levels
  • README typo
  • Improves get_worker doc
  • English typo
  • Improves get_worker that now use passed *args as queue list
  • Removes README toc

File Changes

  • M .gitignore (6)
  • A .travis.yml (10)
  • M README.rst (106)
  • M docs/index.rst (115)
  • M flask_rq.py (102)
  • A tests.py (89)

Patch Links

@nguyenchiencong
Copy link

did you guys manage to make it work? sorry i'm quite a newb in flask. When I'm trying to use the models from my App (with SQLAlchemy), i got either 'working outside of application context' or 'application not registered on db instance and no application bound to current context'. I can't figure out a way to make the job functions recognize the application context. It would be great if you can help. Thanks

@jeanphix
Copy link
Contributor Author

@nguyenchiencong You should consider creating an issue for that with minimal code... For sure it works just fine with sqlalchemy objects.

@nguyenchiencong
Copy link

hi @jeanphix the problem is not with this particular project. I tried to use only rq, and still have the same problem. I think it might be because of how I organize my folders and declare the models in the Flask App. I'm so used to the autoloads of Rails. thanks

@mattupstate
Copy link
Collaborator

@nguyenchiencong You need to create an app context in your tasks since they are executed outside the context of a request. You can do this as such:

@rq.task
def my_task():
    with app.app_context():
        MyModel.query.filter(....)

@mattupstate
Copy link
Collaborator

@jeanphix Can you explain your changes here a bit more? I don't see any practical examples in the tests for the helper functions. Also, most extensions also register themselves on the application and you seem to have removed that.

@jeanphix
Copy link
Contributor Author

jeanphix commented Nov 5, 2012

@mattupstate The README is up to date with examples, also commit titles explain what have been done.

@mattupstate
Copy link
Collaborator

Any idea if the redis python library is thread safe?

Upon further inspection the module level functions all end up calling config_value, which relies on current_app, which requires the call to any methods in this module to have a valid application or request context present. So I'm not sure if you could have code such as this:

from flask import Flask
from flask_rq import task

app = Flask(__name__)

@task
def my_task():
    # long runing task

@app.route('/')
def index():
    my_task()
    return "response..."

If I'm following your code correctly the above would fail. I very well could be wrong though.

@nguyenchiencong
Copy link

Thanks @mattupstate

@jeanphix
Copy link
Contributor Author

Your example code can't work at all cause we can't enqueue main module function call.

Anyway, I've fixed the decorator that can now decorate function outside flask app context. Maybe it should be called @job instead of @task and mimic the one include in rq. I mean with timeout option.

Let me know.

Cheers,

@mattupstate
Copy link
Collaborator

@jeanphix This looks a lot better. One thing that is missing is that the RQ constructor needs to be able to be called without passing any arguments to it. Its a standard pattern for Flask extensions.

@jeanphix
Copy link
Contributor Author

@mattupstate Fixed. You didn't answer about renaming task into job?

@mattupstate
Copy link
Collaborator

Go ahead and change it to job. Job is obviously the language used in the RQ community.

@jeanphix
Copy link
Contributor Author

@mattupstate Done

mattupstate pushed a commit that referenced this pull request Nov 12, 2012
Merge @jeanphix's changes to drastically improve extension
@mattupstate mattupstate merged commit c5e81ed into pallets-eco:master Nov 12, 2012
@mattupstate
Copy link
Collaborator

Thanks so much. I've uploaded the new release to PyPi

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

Successfully merging this pull request may close these issues.

None yet

3 participants