Move labman.primer_set 18S and ITS inserts from db_patch_manual.sql into the populate_prod_db.sql

[wasade]

It is not possible to create ITS or 18S primer plates. The example .gif shows an attempt to create an 18S plate; the same error is observed with ITS. 16S and shotgun appear to work.

An observed traceback is below.

Traceback
Traceback (most recent call last): 
File "/Users/dtmcdonald/miniconda3/envs/labcontrol/lib/python3.7/site-packages/tornado/web.py", line 1697, in _execute result = method(*self.path_args, **self.path_kwargs) 
File "/Users/dtmcdonald/miniconda3/envs/labcontrol/lib/python3.7/site-packages/tornado/web.py", line 3174, in wrapper return method(self, *args, **kwargs) 
File "/Users/dtmcdonald/ResearchWork/software/labman/labman/gui/handlers/process_handlers/primer_working_plate_creation_process.py", line 35, in post creation_date) 
File "/Users/dtmcdonald/ResearchWork/software/labman/labman/db/process.py", line 404, in create check_name = '%s %s' % (primer_set_plates[0].external_id, 
IndexError: list index out of range 

[charles-cowart]

@wasade it looks like this was on localhost, but today we tried it on maptest successfully. I will look into it.

[wasade]

Okay thanks. Maybe just some staging state, might be the initial population script is limited

[AmandaBirmingham]

TL;DR: This is a shortcoming in the db population scripts for our dev environments and not a bug in "real" LabControl instances. We can fix it the hard way by changing the dev db population script (and then the hard-coded ids in practically every freaking unit test) or we can fix it the easy way by changing our policy on how many primer sets of each kind the dev environment really needs to exercise. Read the longer stuff below for details on these two options. I am changing the priority to low as this bug is real but affects only dev environments.

---

Details:

I diagnosed this one. The observed error is limited to LabControl instances running on a database populated by the populate_test_db.sql script--which is to say, pretty much everyone's local dev environment. However, it is NOT reproducible on a LabControl instance running on a database populated by the populate_prod_db.sql script--e.g. maptest (I have verified) and the production labcontrol (I believe but haven't tested bc, well, production).

The issue is simple: db_patch_manual.sql creates the plates (and wells, and containers, and yadda yadda) for the plate maps of 16S and shotgun primer plates, but NOT for 18S or ITS primer plates. If you set up a production db by running populate_prod_db.sql , it adds in the plate maps for 18S and ITS primer sets, but populate_test_db.sql does NOT.

The true fix for this (which would make dev environments actually able to demonstrate the ability to use 18S and ITS primer sets in LabControl) is putting the functionality from populate_prod_db.sql that defines the 18S and ITS primer plate maps into db_patch_manual.sql. HOWEVER, surprisingly, this will be a large-scope task because it will cause changes in the plate ids/container ids/well ids/etc for all downstream sql runs (like populate_test_db.sql)--and most of the unit tests depend on HARD-CODED RECORD IDS.

The only lower-effort "fix" I can think of is to remove the definitions of the 18S and ITS marker gene primer sets from db_patch_manual.sql, leaving only 16S, and moving those 18S/ITS definitions to populate_prod_db.sql. This would have approximately no effect on the db record ids used by the unit tests. It would mean we couldn't test amplicon library prep using 18S/ITS primer sets from a dev environment ... but note that we obviously can't (and have NEVER BEEN ABLE TO) do that anyway.

It is worth thinking about whether we view individual primer sets as something LabControl tests or not. The lab occasionally (though rarely) creates new primer sets (e.g., there have been several iterations of the 16S primer set, although only one is currently reflected in the LabControl db). Do we (a) figure we have to update the LabControl dev environment to include every new primer set the lab invents? If so, that will be a painful if infrequent task due to the hard-coded record ids in the tests. Or do we (b) say "we demonstrate the functionality of LabControl in a dev environment for ONE of each of the two kinds of primer sets it supports (marker gene and shotgun), but not every new one that is created".

I kinda vote for (b), but if we choose (a), this is probably the time to seriously refactor the hard-coded ids in the unit tests, because I have done this id shuffle at least 3 times during the LabControl development, and it is a remarkable sink for developer time that does not directly improve the customer-facing software.

[wasade]

That is frustrating, thank you for tracking down @AmandaBirmingham. I'm inclined for (b) as well. I think the lower-effort fix here makes sense. This is not ideal, but absolutely agree that it is difficult to justify the developer expense to resolve this for a dev environment.

[AmandaBirmingham]

@charles-cowart Please holler now if you DISAGREE with option (b) discussed above. Otherwise I'ma redefine this issue to be moving 18S and ITS primer set definitions into the production db set-up sql.

I believe this is the right decision in part because I checked the history, and 18S and ITS were historically NOT part of the LabControl dev environment--they only came in shortly before soft launch with https://github.com/jdereus/labman/pull/319 . I wrote that PR, and actually it just aimed to get them into the production environment, but I didn't think through the full implications of putting their definitions in db_patch_manual.sql instead of populate_prod_db.sql :-/

[antgonza]

Agree on b; only suggestion will be to add this to the CHANGELOG or somewhere in case future devs need to figure out this decision. BTW do LabControl have a CHANGELOG and is it up to date?

[AmandaBirmingham]

As far as I know, LabControl does not have a CHANGELOG.

[charles-cowart]

No disagreements w/option b. I'll add a CHANGELOG.