Skip to content

fix: remove app spaces in fasta header - #184

Merged
vtnphan merged 1 commit into
devfrom
sbp-671
Sep 29, 2026
Merged

vtnphan merged 1 commit into
devfrom
sbp-671

Conversation

@vtnphan

@vtnphan vtnphan commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Pull Request

Summary

SBP-671 Remove fasta header spaces in samplesheet (WISPS workflow)

Changes

  • Added sanitize_filename() in app/services/s3.py: strips any character outside [A-Za-z0-9._-] to _, falling back to "file" if empty; applied to S3 upload keys.
  • build_gadi_input_path() (globus_transfer.py) now sanitizes the filename before building the Gadi input path.
  • WispsSequenceItem.id (interaction_screening.py) now enforces the same ^[A-Za-z0-9._-]+$ pattern via Pydantic, rejecting unsafe IDs at the API boundary.
  • No breaking changes; only previously-unsafe inputs are affected.

How to Test

  1. Submit an interaction-screening request with a sequence id containing [, ], or spaces and confirm it's rejected with a validation error.
  2. Upload a file with an unsafe filename (spaces/brackets) and confirm the resulting S3 key/Gadi path has those characters replaced with _.
  3. uv run pytest tests/test_schemas.py tests/test_services_s3.py — new tests cover the pattern validation and sanitizer.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added or updated documentation where necessary
  • I have run linting and unit tests locally
  • The code follows the project's style guidelines

@vtnphan
vtnphan marked this pull request as ready for review September 29, 2026 00:19
@vtnphan
vtnphan merged commit cf7c872 into dev Sep 29, 2026
2 checks passed
@vtnphan
vtnphan deleted the sbp-671 branch September 29, 2026 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants