Fixes and improvements for galaxy.json.#2697
Conversation
1932fc1 to
000b50f
Compare
|
Failed framework test seems legit. |
…hange. This has been broken since 16.04 I guess? Maybe retry internally means it hasn't been an issue?
Based on the change - I guess this must have been broken a long time ago - so maybe it isn't used? A subsequent change will add a test though - so hopefully there will be no regressions of this again.
…puts. I call explicit outputs galaxy.json entries of type="dataset" (as opposed to type="new_primary_dataset" - which I call discovered datasets). Previously galaxy.json could set the name and extension of these datasets. I've added a test case to verify this and a small tweak to the tool test framework to allow easy testing of such metadata setting. In order to bring explicit output metadata setting closer to parity with discovered dataset metadata setting, I've added the ability to define 'dbkey' and 'info' entries in galaxy.json for these outputs.
Seems like a race condition but I can't determine why there would be one here - this check should help determine if it is a race condition or something else.
Previously some sort of test on the base file the primary datasets were keyed on was required.
Also update the metadata test comparison to support non-textual values (e.g. floating point and integer metadata fields).
3aea7ba to
9ae3339
Compare
|
@nsoranzo Yeah - it was an hda id being conflated with a dataset id - this wasn't a problem in one-off testing but tested with the rest of the tests it is a problem I think - or maybe it was a type problem - either way I fixed both and verified the tool under question works with postgres now. Thanks for the catch. |
|
@bgruening and @nsoranzo - thanks for the reviews - either of you willing to merge? |
lib/galaxy/jobs/__init__.py
Outdated
| try: | ||
| context_value = context[context_key] | ||
| setattr(dataset, context_key, context_value) | ||
| except Exception: |
There was a problem hiding this comment.
@jmchilton I know this was pre-existing, but is the expected Exception here only KeyError ?
Maybe sth. like
if context_key in context:
context_value = context[context_key]
setattr(dataset, context_key, context_value)
would be better ?
There was a problem hiding this comment.
It is an ExpressionContext not a dictionary - I just looked into the code and it looks like it would throw a KeyError and it would respect in. I'l make the change.
There was a problem hiding this comment.
Updated it - thanks for the suggestion.
... see galaxyproject#2697 (comment) for context.
|
Thanks @jmchilton, I'm sure this will come in handy ! |
Update:
I rebased a unit test fix, and pushed out two new commits - one that fixed a very old bug that wouldn't allow tools to 'only' test discovered datasets and one that added a new tool documenting specifying arbitrary metadata in outputs.
The failing framework test seems legitimate but runs fine locally, I wonder if the test framework has a race condition when using postgres.