Unify metadata creation #1032
Unify metadata creation #1032
Conversation
|
Do not merge this, I'm adding a test as per @antgonza request |
|
This includes the commits from #1033. Once that one gets merged, I will pull from upstream/master and push again, so the commits will go away and only the new code is being shown here. I've added a test for the |
|
Changes Unknown when pulling aff06a3 on josenavas:unify-metadata-creation into * on biocore:master*. |
|
👍 |
|
Changes Unknown when pulling 2ae8609 on josenavas:unify-metadata-creation into * on biocore:master*. |
|
Ready for another review/merge if there are no comments! |
| headers = list(md_template.keys()) | ||
|
|
||
| # Get the required columns from the DB | ||
| db_cols = get_table_cols(cls._table, conn_handler) |
There was a problem hiding this comment.
This looks looks like the function needs the "am I instantiating the base class" test.
There was a problem hiding this comment.
I was thinking on that, but this is a private function and it should be only used in the create functions, that already have that test. If you feel strong about it, I can add it though...
There was a problem hiding this comment.
Would probably be good to add as a safety layer, since it will also mean a more interpretable error.
|
Few comments. |
|
@squirrelo, @antgonza already gave his 👍 if you think the code looks good can you merge? Thanks! |
The code that was adding SQL statements to the queue was duplicated between the PrepTemplate and the SampleTemplate. I moved this code to the base class so future changes in this code are centralized...
Fixes #440