Offloading file creation form the webserver #2004
Conversation
There was a problem hiding this comment.
Looks good, one misspelling. There are no direct tests for the new methods added, like wait_for_processing_job and _job_completer, which I think is fine as they are being tested in the full run. My only concern is that you are missing areas in the full code that need we need to add wait_for_processing_job. I think the best way to be sure is to deploy in the test env and check that is working. @ElDeveloper or @wasade, could you review?
|
|
||
|
|
||
| def wait_for_processing_job(job_id): | ||
| """Waits unitl a processing job is completed |
| # We agreed before that qiita db should never import from | ||
| # qiita_ware. However, this is part of the rest API and I think it | ||
| # is acceptable to import from qiita_ware, specially to offload | ||
| # processed to the ipython cluster |
There was a problem hiding this comment.
processed -> processing (?)
| job = qdb.processing_job.ProcessingJob(job_id) | ||
| try: | ||
| job.complete(payload_success, artifacts, error) | ||
| except Exception as e: |
There was a problem hiding this comment.
Would it be worth to save the traceback itself? Or is this information presented to the user?
There was a problem hiding this comment.
It may be shown to the user, but if it fails at this point is better for us if it is actually store the traceback, so thanks for pointing that out!
|
Looks great, thanks for making the changes @josenavas! |
I think there was an issue open but I can't find it - if somebody knows which one it is, it will be nice if it can be linked here.
Already noted in the code but explaining it here to. It is moving this processing to the ipyhon cluster, which means that qiita_db.handlers is importing from qiita_ware. We had a rule to not import from qiita_ware in qiita_db, but I think in this case is acceptable given that it's the webserver.
This may point out that we probably want to move this handlers out of qiita_db, and create a new module called qiita_rest.