search

LBNL-ETA / BEDES-Manager

Other
1 stars 0 forks source link

ensure list of units in database matches "Unit Of Measure" constrained list #200

Closed TravisWalterLBNL closed 2 years ago

TravisWalterLBNL commented 3 years ago

What is the status on this issue? Did we do a manual fix for now, but have not yet implemented a permanent fix?

ccobb-lbl commented 3 years ago

@wizonesolutions , I still see this in todo in our JIRA, but I thought we had everything pretty much wrapped up except for a couple points Diego is working on. Please share an update when you can. @TravisWalterLBNL, if it's not already done (but in the incorrect status), it should be done shortly.

wizonesolutions commented 3 years ago

This is in code review now. It's not user-testable since we've already addressed the missing units on stage/production.

I suggest we leave this open (but maybe assigned to @TravisWalterLBNL) until it is time to update the dictionary to v2.5, which is being worked on. If that goes well, we can close this. Or, we can close it and open a separate issue if we run into any import issues with 2.5.

Remember, the use case that triggered this was "unit defined in Unit Of Measure but not used by any BEDES-defined terms, only by app terms." We ran into it when importing espm.csv.

I could also check that it's working manually against v2.5 once we have a 2.5.

TravisWalterLBNL commented 3 years ago

Let's leave this open and assigned to @caseycobb. If the v2.5 import goes as planned, we'll close this issue then.

TravisWalterLBNL commented 2 years ago

It seems like something might be wrong with this. I was trying to test this issue by making an application that used every legal value of unit, but when I try to import from .csv, I get the error message "Incorrect BedesTermUnit. Term=($)", seemingly indicating that something is wrong with the unit for the first app term "$". However, "$" is a legal unit and should be accepted. If I remove the corresponding line from the .csv file, everything else imports without issue. Strangely, if I change the unit for that first app term from "$" to something that should not be accepted (e.g., "abcxyz"), I get a different error message: "Error: Not an official BEDES unit. Term = $".

wizonesolutions commented 2 years ago

Simple issue: the id of $ happened to be 0, and the code wasn't validating precisely enough. Fix deploying to staging. Check in 15-20 mins.

TravisWalterLBNL commented 2 years ago

OK, this looks good on staging. I'll close this issue after testing on production.

TravisWalterLBNL commented 2 years ago

Fixed on production.