Transactionize the analysis and job objects #1332
Conversation
|
This have the same failure as #1326 |
…r-1318-analysis-job
|
This looks fine. 👍 |
| "ans.status = %s".format(cls._table)) | ||
| return {x[0] for x in conn_handler.execute_fetchall(sql, (status,))} | ||
| with TRN: | ||
| sql = """SELECT analysis_id |
There was a problem hiding this comment.
This indentation is odd, and this SQL should also be changed to use USING. Standard SQL indentation is:
SELECT analysis_id FROM qiita.{0}
JOIN qiita.{0}_status USING (analysis_status_id)
WHERE ans.status = %sThere was a problem hiding this comment.
@squirrelo can you point to the "standard" that you're talking about? There is no standard, only coding guidelines (that we can user or not) but None of the ones that I've seen use your indentation. Usually is common practice to put the FROM in the next line under the SELECT, not following it (quick googling will show this to you...)
I can agree on adding the JOIN in a single line but it didn't fit. Will change for USING.
|
Some SQL cleanup that needs to be done (not your fault but good to do since this pull request is going in to clean things up anyway). Other than that, looks good. |
|
@squirrelo comments addressed |
|
👍 |
Transactionize the analysis and job objects
The analysis and job object now use transactions.
I've been removing some SQL queries to use
convert_to_idin a couple of places.