Skip to content

Enable attaching sample sheet to landing requests#21489

Merged
mvdbeek merged 6 commits intogalaxyproject:devfrom
mvdbeek:add-sample-sheet-to-landing-requests
Jan 13, 2026
Merged

Enable attaching sample sheet to landing requests#21489
mvdbeek merged 6 commits intogalaxyproject:devfrom
mvdbeek:add-sample-sheet-to-landing-requests

Conversation

@mvdbeek
Copy link
Copy Markdown
Member

@mvdbeek mvdbeek commented Dec 18, 2025

Both for data landing and workflow landing requests

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek mvdbeek force-pushed the add-sample-sheet-to-landing-requests branch 11 times, most recently from ca7fb09 to 25ccc4a Compare December 18, 2025 19:18
@jmchilton
Copy link
Copy Markdown
Member

I'm not opposed to any of this and I do appreciate you getting it to work - I think maybe I wish the validation felt more holistic. Like I assume we're not validating collection_type any of those places and we're not validating record types (if there is record type validation code anywhere). But I can create an issue to revisit this and clean it up if I'm unhappy with it - I know you're in a hurry and you've done way more work than you expected to get it working and I really appreciate all of that.

@mvdbeek
Copy link
Copy Markdown
Member Author

mvdbeek commented Dec 19, 2025

I can work on the validation, I agree that's just as important as the functionality. I think feature-wise we're probably good for the planned demo in January, so there's no particular rush ... I can always deploy from a branch for test.galaxyproject.org

mvdbeek pushed a commit to mvdbeek/galaxy that referenced this pull request Jan 12, 2026
This commit addresses the feedback from PR galaxyproject#21489 to make validation
feel more holistic by consolidating validation into Pydantic models
and applying it consistently across the codebase.

Changes:
- Consolidate SampleSheetColumnDefinition: Replace TypedDict with
  Pydantic model in schema.py, providing runtime validation at API
  boundaries. Add backwards-compatible dict-like access methods.

- Extend collection_type validation: Update _check_collection_type()
  to validate all collection types (list, paired, paired_or_unpaired,
  sample_sheet, record). Flat types (sample_sheet, record) cannot be
  nested with other types.

- Add RecordField model: New Pydantic model for record collection
  field definitions with name and type validation.

- Fix landing request validation: Remove silent error swallowing in
  validate_workflow_request_state(). ValidationError is now properly
  converted to RequestParameterInvalidException.

- Add cross-validation: BaseCollectionTarget now validates that:
  * record collections require 'fields'
  * sample_sheet collections require 'column_definitions'

- Update imports: sample_sheet_util.py, sample_sheet_workbook.py,
  and dataset_collections service now import from the canonical
  schema.py location.
@mvdbeek
Copy link
Copy Markdown
Member Author

mvdbeek commented Jan 12, 2026

Did you have something like https://github.com/galaxyproject/galaxy/compare/dev...mvdbeek:claude/enhance-sample-validation-TjCue?expand=1 in mind ? I think I'd make that a follow up, I hope that's ok ?

@jmchilton
Copy link
Copy Markdown
Member

Did you have something like https://github.com/galaxyproject/galaxy/compare/dev...mvdbeek:claude/enhance-sample-validation-TjCue?expand=1 in mind ? I think I'd make that a follow up, I hope that's ok ?

That is not what I was imagining - it uses Pydantic so much more intelligently than what I was thinking and will probably produce much clearer errors, much more consistently. Very nice enhancement - thank you - totally fine with me if it comes in a subsequent PR.

@mvdbeek mvdbeek force-pushed the add-sample-sheet-to-landing-requests branch from 8d002de to 55b76db Compare January 12, 2026 18:36
@mvdbeek mvdbeek marked this pull request as ready for review January 12, 2026 18:38
@github-actions github-actions Bot added this to the 26.0 milestone Jan 12, 2026
@mvdbeek mvdbeek merged commit 1ca7ad8 into galaxyproject:dev Jan 13, 2026
59 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants