Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client Settings Policy Enhancement Proposal #1692

Merged
merged 20 commits into from
Mar 25, 2024

Conversation

kate-osborn
Copy link
Contributor

Proposed changes

Problem: Need design for specifying client settings like client_max_body_size

Solution: Add enhancement proposal introducing ClientSettingsPolicy.

Enhancement Proposal: #1632

Closes #1603

NOTE: The diagrams are a little hard to see in dark mode.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

NONE

@kate-osborn kate-osborn added the enhancement-proposal Enhancement Proposal issue label Mar 14, 2024
@kate-osborn kate-osborn requested review from a team as code owners March 14, 2024 15:56
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 14, 2024
Copy link
Contributor

@sjberman sjberman left a comment

Choose a reason for hiding this comment

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

One note with the images; when using dark mode, the text and arrows between boxes is difficult to see.

docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Show resolved Hide resolved
Copy link
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

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

LGTM, read through all of it and it mostly made sense, won't approve for now as I will let the others get to it first

docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

the diagrams are huge help 👍 nicely done

docs/proposals/client-settings.md Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
docs/proposals/client-settings.md Outdated Show resolved Hide resolved
@kate-osborn kate-osborn removed the documentation Improvements or additions to documentation label Mar 22, 2024
@kate-osborn kate-osborn force-pushed the docs/proposals/client-settings branch from ec7776b to 079a900 Compare March 25, 2024 18:25
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 25, 2024
@kate-osborn
Copy link
Contributor Author

One note with the images; when using dark mode, the text and arrows between boxes is difficult to see.

Should be fixed now.

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

kate-osborn and others added 8 commits March 25, 2024 15:22
Co-authored-by: Saylor Berman <s.berman@f5.com>
Co-authored-by: Saylor Berman <s.berman@f5.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
@kate-osborn kate-osborn force-pushed the docs/proposals/client-settings branch from 5e78f3f to 1412269 Compare March 25, 2024 21:24
@kate-osborn kate-osborn removed the documentation Improvements or additions to documentation label Mar 25, 2024
@kate-osborn kate-osborn merged commit 3ee65f6 into nginxinc:main Mar 25, 2024
40 checks passed
amimimor pushed a commit to amimimor/nginx-gateway-fabric that referenced this pull request Apr 3, 2024
Problem: Need design for specifying client settings like client_max_body_size

Solution: Add enhancement proposal introducing ClientSettingsPolicy.
@lucacome lucacome added the documentation Improvements or additions to documentation label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement-proposal Enhancement Proposal issue
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Design ClientSettingsPolicy
5 participants