Skip to content
This repository was archived by the owner on May 6, 2024. It is now read-only.

feat: add nginx server names hash override [BB-5512] - #6682

Merged
nadeemshahzad merged 1 commit into
openedx-unsupported:masterfrom
open-craft:kaustav/override_default_hash_bucket_size
Feb 17, 2022
Merged

feat: add nginx server names hash override [BB-5512]#6682
nadeemshahzad merged 1 commit into
openedx-unsupported:masterfrom
open-craft:kaustav/override_default_hash_bucket_size

Conversation

@kaustavb12

@kaustavb12 kaustavb12 commented Feb 13, 2022

Copy link
Copy Markdown
Contributor

Configuration Pull Request

Description

This PR adds support for optional configuration of nginx server_names_hash_bucket_size in case length of the hostname exceeds the default hash bucket size.

Supporting information

BB-5512

TODO

Cherry-pick to open-release/lilac.master and open-release/maple.master once this is merged.

Make sure that the following steps are done before merging:

  • A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? If so:
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Feb 13, 2022
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @kaustavb12! I've created OSPR-6454 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@farhaanbukhsh farhaanbukhsh 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.

👍

  • ✅ I tested this, checked deployed changes on OCIM.
  • ✅ I read through the code
  • ❌ I checked for accessibility issues
  • ✅ Includes documentation
  • ❌ I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@natabene

Copy link
Copy Markdown
Contributor

@kaustavb12 Thank you for your contribution. Please let me know once it is ready for our review.

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Feb 14, 2022
@kaustavb12

Copy link
Copy Markdown
Contributor Author

@natabene This PR is ready for your review

@openedx-webhooks openedx-webhooks added awaiting prioritization and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Feb 16, 2022
@nadeemshahzad
nadeemshahzad merged commit 7d4476d into openedx-unsupported:master Feb 17, 2022
@openedx-webhooks

Copy link
Copy Markdown

@kaustavb12 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

github-actions Bot added a commit that referenced this pull request Apr 2, 2022
Configuration Pull Request
---
## Description

cherry picked from commit [`59cbc50`](59cbc50) ([PR#6682](#6682))

This PR adds support for optional configuration of `nginx server_names_hash_bucket_size` in case length of the hostname exceeds the default hash bucket size.

## Supporting information

[BB-5512](https://tasks.opencraft.com/browse/BB-5512)


Make sure that the following steps are done before merging:

  - [ ] A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  - [ ] Are you adding any new default values that need to be overridden when this change goes live? If so:
    - [ ] Update the appropriate internal repo (be sure to update for all our environments)
    - [ ] If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    - [ ] Add an entry to the CHANGELOG.
  - [ ] If you are making a complicated change, have you performed the proper testing specified on the [Ops Ansible Testing Checklist](https://openedx.atlassian.net/wiki/spaces/SRE/pages/28967861/Ops+Ansible+Testing+Checklist)?  Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  - [ ] Think about how this change will affect Open edX operators.  Have you updated the wiki page for the next Open edX release?
github-actions Bot added a commit that referenced this pull request Apr 2, 2022
Configuration Pull Request
---
## Description

cherry picked from commit [`59cbc50`](59cbc50) ([PR#6682](#6682))

This PR adds support for optional configuration of `nginx server_names_hash_bucket_size` in case length of the hostname exceeds the default hash bucket size.

## Supporting information

[BB-5512](https://tasks.opencraft.com/browse/BB-5512)


Make sure that the following steps are done before merging:

  - [ ] A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  - [ ] Are you adding any new default values that need to be overridden when this change goes live? If so:
    - [ ] Update the appropriate internal repo (be sure to update for all our environments)
    - [ ] If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    - [ ] Add an entry to the CHANGELOG.
  - [ ] If you are making a complicated change, have you performed the proper testing specified on the [Ops Ansible Testing Checklist](https://openedx.atlassian.net/wiki/spaces/SRE/pages/28967861/Ops+Ansible+Testing+Checklist)?  Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  - [ ] Think about how this change will affect Open edX operators.  Have you updated the wiki page for the next Open edX release?
@0x29a
0x29a deleted the kaustav/override_default_hash_bucket_size branch May 3, 2022 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

merged open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants