-
Notifications
You must be signed in to change notification settings - Fork 17
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
Remove HTTP feature flags #2918
Conversation
dc92468
to
7f544e4
Compare
The rendering of recommended related links was conditional on a feature flag, which is set via a header within our Fastly VCL. As an outcome of RFC-163, we are trying to simplify our CDN configuration by removing functionality that’s no longer used, or by moving logic to other parts of the stack. This change was suggested in a comment on RFC-163: "I think the flag for Whitehall recommended links should be removed. It was put in as a safety mechanism for the introduction of the links, in case something really inappropriate was found. There are ways to manually override the links now, and we've never used the feature flag." Related content is component is defined in govuk_publishing_components gem and this is where the overriding is happening (see: https://github.com/alphagov/govuk_publishing_components/blob/5d713ff637de14bec6abc784c35ee63a0cbc2c75/lib/govuk_publishing_components/presenters/content_item.rb#L70-L74) It's now recommended to create feature flags as environment variables in helm charts. See the docs: https://github.com/alphagov/govuk-developer-docs/blob/33f1f0434aa988419df0687a829367e3f769816f/source/kubernetes/manage-app/set-env-var/index.html.md?plain=1#L7-L21 The value can then be read in Rails config: ``` config.unreleased_features = ENV["UNRELEASED_FEATURES"].present? ```
7f544e4
to
ba01552
Compare
As an outcome of RFC-163, we are trying to simplify our CDN configuration by removing functionality that’s no longer used, or by moving logic to other parts of the stack. This change was suggested in a comment on RFC-163 (alphagov/govuk-rfcs#163 (comment)): "I think the flag for Whitehall recommended links should be removed. It was put in as a safety mechanism for the introduction of the links, in case something really inappropriate was found. There are ways to manually override the links now, and we've never used the feature flag." The application logic dependent on this header is removed in alphagov/government-frontend#2918 It's now recommended to create feature flags as environment variables in helm charts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice one!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a little bit confused by the PR description - is the info about how to set feature flags via env vars just for information? Or are we needing to do that here? I am assuming not - because there's a UI to set an override now. But just wanted to make sure I wasn't missing something
Yes, that's just FYI and to rationalise why I'm removing the entire HTTP headers feature flags, not just the recommended links one. Sorry if it wasn't clear. |
The rendering of recommended related links was conditional on a feature flag,
which is set via a header within our Fastly VCL.
As an outcome of RFC-163, we are trying to simplify our CDN configuration by
removing functionality that’s no longer used, or by moving logic to other parts
of the stack.
This change was suggested in a comment on RFC-163:
'Related content' component is defined in govuk_publishing_components gem and
this is where the overriding is happening (see presenters/content_item.rb)
It's now recommended to create feature flags as environment variables in helm
charts. See the docs
The value can then be read in Rails config:
Trello
Related PRs
Follow these steps if you are doing a Rails upgrade.