Skip to content

[#79,#484] Limit parallel-write streams across all clients (main) - #501

Merged
alanking merged 7 commits into
irods:mainfrom
korydraughn:79.m
Jun 17, 2026
Merged

[#79,#484] Limit parallel-write streams across all clients (main)#501
alanking merged 7 commits into
irods:mainfrom
korydraughn:79.m

Conversation

@korydraughn

@korydraughn korydraughn commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

All tests pass.

korydraughn
korydraughn commented Jun 15, 2026
View reviewed changes
Comment thread test/test_irods_http_api.py Outdated
alanking
alanking reviewed Jun 16, 2026
View reviewed changes

@alanking alanking left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems reasonable

Comment thread README.md
Comment thread endpoints/data_objects/src/main.cpp
Comment thread test/test_irods_http_api.py
Comment thread test/test_irods_http_api.py
@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Rebased to resolve conflict.

@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

All non-OIDC tests pass.

alanking
alanking reviewed Jun 17, 2026
View reviewed changes

@alanking alanking left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just noting that we didn't just rename an existing configuration, but also added a new one which has the old name of the existing configuration. Will need to put that in big letters for the release notes

Comment thread core/src/main.cpp
@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Just noting that we didn't just rename an existing configuration, but also added a new one which has the old name of the existing configuration.

Correct. The inclusion of _active_ in the config property's name didn't add any value. We are redefining what an existing property means though.

Will need to put that in big letters for the release notes

Agreed.

alanking
alanking reviewed Jun 17, 2026
View reviewed changes

@alanking alanking left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay, squash to taste

@korydraughn korydraughn mentioned this pull request Jun 17, 2026
Closed
@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Squashed. Please eyeball commit messages for typos, grammar.

alanking
alanking reviewed Jun 17, 2026
View reviewed changes

@alanking alanking left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just had one suggestion on wording...

On "Limit parallel-write streams across all clients":
"Exceeding this limit..." might sound better than "Violating this limit..."

@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Updated commit message.

@alanking

alanking commented Jun 17, 2026

Copy link
Copy Markdown

Oops, # it

@korydraughn
[irods#79,irods#484] Add test for parallel-write stream limit
4a36c36
@korydraughn
[irods#79,irods#484] Limit parallel-write streams across all clients
d4500be 
This commit adds a new configuration property and re-purposes an
existing one.

  - max_number_of_parallel_write_streams

    Limits how many streams can be reserved by the HTTP API, across all
    clients. Exceeding this limit will result in an HTTP status code of
    503 (Service Unavailable).

    Before this commit, this property served the same purpose as the new
    property (shown below).

  - max_number_of_streams_per_parallel_write_handle

    Limits how many streams can be associated to a single parallel-write
    handle. Exceeding this limit will result in an HTTP status code of
    400 (Bad Request).

Administrators will need to update their configuration on upgrade.
@korydraughn
[irods#43] Remove unused argument from log invocations
b73b150
@korydraughn
[irods#43] Resolve clang-tidy issues
08c67c5
@korydraughn
[irods#43] Adjust name of global variables
cf51ffc
@korydraughn
[irods#43] Add test showing parallel_write_init enforces per-handle l…
e917e92 
…imit
@korydraughn
[irods#79,irods#484] Enforce post jsonschema validation requirements
afe881b
@korydraughn korydraughn changed the title [#79,#484] Limit active parallel-write streams across all clients (main) [#79,#484] Limit parallel-write streams across all clients (main) Jun 17, 2026
@korydraughn

korydraughn commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Added pounds.

@alanking
alanking merged commit fae892e into irods:main Jun 17, 2026
13 checks passed
@korydraughn
korydraughn deleted the 79.m branch June 17, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@alanking alanking alanking approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@korydraughn @alanking