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

Specify GET method when submitting task for CSV load so that worker doesn't return 405 #7

Merged
merged 1 commit into from
Feb 15, 2017

Conversation

brenns10
Copy link
Contributor

@brenns10 brenns10 commented Feb 15, 2017

Hello! I battled with a bit of an issue setting up yelp-love and using the CSV import feature. Apparently the CSV import submits a task to a queue in the worker module, which does the actual import.

The first issue I ran across was that the request for /tasks/employees/load/csv did not seem to get routed to the worker module. I addressed that by uploading the dispatch.yaml file (I used a command found in #5).

The next issue I encountered (which this PR addresses) is that the task would return 405 Method Not Allowed. Turns out that the task function only allows a GET method, but when the task is submitted, the taskqueue.add() function defaults to POST (docs, grep for default to post).

The options were either add POST to the list of allowed methods for load_employees_from_csv, or specify GET in the task submission. In this PR I decided to specify GET in the task submission, since all of the neighboring tasks in views/task.py seemed to be GET only. However, having some methods be GET and some be POST seems like a liability, since there is other code that uses taskqueue.add without specifying the method, and it works fine since those tasks use POST (see logic/love.py and logic/event.py). It may make sense to use POST for all the tasks, if only for the benefit that the next time somebody types taskqueue.add they don't need to know which method type their task uses.

@sjaensch sjaensch merged commit 3394bc0 into Yelp:master Feb 15, 2017
@sjaensch
Copy link
Contributor

@brenns10 Thanks for your contribution!

@brenns10 brenns10 deleted the fix-csv-import branch February 18, 2017 19:42
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.

2 participants