Project

General

Profile

Actions

Bug #21841

closed

check on 'workflowName' and 'container' property keys in vocabulary

Added by Peter Amstutz about 2 months ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
-

Subtasks 1 (0 open1 closed)

Task #21856: Review 21841-missing-vocabResolvedPeter Amstutz06/06/2024Actions
Actions #1

Updated by Peter Amstutz about 2 months ago

workflowName is set by Workbench 2 when when launching a workflow (run-process-panel-actions.ts). It is the name field of the workflow object that gets recorded as template_uuid and used to provide a human-friendly name instead of displaying the UUID (it could look up the record to get the current name, but doesn't.)

container was set on intermediate collections created by older versions of arvados-cwl-runner prior to Arvados 2.6.0. This was a typo, the field should have been called "container_uuid". It was fixed in #19960.

Actions #2

Updated by Peter Amstutz about 2 months ago

  • Status changed from New to In Progress
Actions #3

Updated by Peter Amstutz about 2 months ago

to do -> find where the built-in vocabulary validation happens and see if these are included.

Actions #4

Updated by Brett Smith about 2 months ago

Peter Amstutz wrote in #note-3:

to do -> find where the built-in vocabulary validation happens and see if these are included.

source:sdk/go/arvados/vocabulary.go and no, they are not.

They need to be documented in source:doc/api/properties.html.textile.liquid too.

Actions #6

Updated by Peter Amstutz about 2 months ago

  • Target version changed from Development 2024-06-05 sprint to Development 2024-06-19 sprint
Actions #7

Updated by Peter Amstutz about 2 months ago

  • Category set to API
Actions #8

Updated by Peter Amstutz about 2 months ago

21841-missing-vocab @ 7786e414318b20123a5435ea0f6403ffe95bccf7

merged main on account of a failing test.

developer-run-tests: #4277

Actions #9

Updated by Peter Amstutz about 2 months ago

21841-missing-vocab @ 7786e414318b20123a5435ea0f6403ffe95bccf7

developer-run-tests: #4277

  • All agreed upon points are implemented / addressed.
    • 'workflowName' and 'container' are now documented and accounted for in the system vocabulary in sdk/go/arvados/vocabulary.go
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • n/a
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • passes existing tests
  • Documentation has been updated.
    • keys are now listed in doc/api/properties.html.textile.liquid
  • Behaves appropriately at the intended scale (describe intended scale).
    • should have no impact on scaling
  • Considered backwards and forwards compatibility issues between client and server.
    • this should improve compatibility for clusters that use 'strict vocabulary' as they don't need to include these Arvados system keys in their vocabulary
  • Follows our coding standards and GUI style guidelines.
    • yes
Actions #10

Updated by Brett Smith about 2 months ago

Peter Amstutz wrote in #note-9:

21841-missing-vocab @ 7786e414318b20123a5435ea0f6403ffe95bccf7

My only request is that we please s/a-c-r/arvados-cwl-runner/g in doc/api/properties.html.textile.liquid. I'm totally fine with the abbreviation in comments and developer documentation, but I don't think it's front of mind for users, and I don't think the abbreviation really contributes to readability, so seems better to just not.

Other than that, lgtm, thanks.

Actions #11

Updated by Peter Amstutz about 2 months ago

Brett Smith wrote in #note-10:

Peter Amstutz wrote in #note-9:

21841-missing-vocab @ 7786e414318b20123a5435ea0f6403ffe95bccf7

My only request is that we please s/a-c-r/arvados-cwl-runner/g in doc/api/properties.html.textile.liquid. I'm totally fine with the abbreviation in comments and developer documentation, but I don't think it's front of mind for users, and I don't think the abbreviation really contributes to readability, so seems better to just not.

I agree 100% and would have made the same comment myself, thank you for calling that out.

Actions #12

Updated by Peter Amstutz about 2 months ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF