Create analysis from cart #1041
Conversation
…reate-analysis-from-cart
|
Ready for review |
| dflt_id = owner.default_analysis | ||
| sql = """INSERT INTO qiita.{0} | ||
| (email, name, description, analysis_status_id) | ||
| VALUES (%s, %s, %s, 3) |
There was a problem hiding this comment.
can we avoid hardcoding IDs? We never now when these can change. Using the convert_to_id function should make the trick.
There was a problem hiding this comment.
convert_to_id doesn't currently work with status tables, hence issue #292. I have updated the convert_to_id function to work around the issue for now though.
There was a problem hiding this comment.
Aaaand that update breaks all the things. Using a specific SQL statement here and tagged with issue ID.
|
Just a small comment. |
squirrelo
force-pushed
the
create-analysis-from-cart
branch
from
ac92e6a to
f06eb57
Compare
|
Changes made and ready for another review. |
| description, status_id)) | ||
| # MAGIC NUMBER 3: command selection step | ||
| # needed so we skip the sample selection step | ||
| sql = """INSERT INTO qiita.analysis_workflow |
There was a problem hiding this comment.
I just noticed this. Why is the analysis_workflow table needed? Is it only handling a column called step, and there is no further control on it, so it looks like that column can be included in the analysis object???
There was a problem hiding this comment.
The table was supposed to be a housekeeping table to track what step the analysis is on and any other things that needed tracking during analysis creation. I guess it turns out we didn't need as much housekeeping as originally thought.
There was a problem hiding this comment.
Yeah, I think we are keeping track of everything else in the multiple tables that build up the analysis and the step just limits what can be done on the analysis. I've created a new issue #1052 and I assigned to beta. Later we can decide to exactly which alpha release it belongs, but I don't think it is either 0.1 or 0.2. Thanks for the feedback!
|
@squirrelo just one comment that should not block the PR, but I'd like to get an answer to check if we actually need to open an issue for that, as it looks like that table is not needed at all. |
|
👍 |
|
Can this get a second reviewer? |
|
I'll do this in a bit. |
This adds the
from_defaultflag to the Analysis object, allowing you to create a new analysis from previously selected samples. This also modifies the analysis creation to use queues.This pull request relies on #1038 so please merge that one first.Merged. Many of the file changes are from that, onlyAnalysis.create()and thetest_analysis_from_defaulttest function were modified in this pull request.