Skip to content

Fix various STOW-RS issues#57

Merged
nickamzol merged 5 commits into
mainfrom
nickamzol/push-kxlonzpwwvkm
May 7, 2026
Merged

Fix various STOW-RS issues#57
nickamzol merged 5 commits into
mainfrom
nickamzol/push-kxlonzpwwvkm

Conversation

@nickamzol
Copy link
Copy Markdown
Member

This pull request addresses several STOW-RS issues that were reported in #55 and #56.

  • Improved error handling in the STOW-RS service handler
    • Return status code 413 "Payload too large" if the request body exceeds the configured max-upload-size.
  • The association pool no longer leaks semaphore permits when the association is rejected (GH-56).
  • Added some integration tests for STOW-RS, testing the happy path and the problematic cases

@nickamzol nickamzol self-assigned this May 5, 2026
@nickamzol nickamzol added the service:stow-rs This issue affects the STOW-RS service label May 5, 2026
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread src/api/stow/service.rs Fixed
Comment thread tests/common/mod.rs Fixed
Comment thread tests/stow.rs Fixed
Comment thread tests/common/mod.rs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion how we test against external systems (from our BitBucket pipelines):

  services:
    orthanc: # DICOMweb server for testing
      memory: 256
      image: jodogne/orthanc-plugins

We usually let Orthanc or other docker services run as part of the pipeline. GH has a similar feature https://docs.github.com/en/actions/tutorials/use-containerized-services/use-docker-service-containers

Running tests locally would be less convenient then however

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the testcontainers approach wouldn’t work for you?
It should work in most CI environments, as long as Docker is available at test time.

As you can see here, the GitHub Actions runner successfully executed the integration tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was just an idea to reduce rust code that needs to be maintained, but both variants work of course :)

@nickamzol nickamzol force-pushed the nickamzol/push-kxlonzpwwvkm branch from b987dcc to 1ec72e8 Compare May 7, 2026 08:11
@nickamzol nickamzol merged commit b212091 into main May 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

service:stow-rs This issue affects the STOW-RS service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants