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

param: Enable absolute bits parameters #4016

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Conversation

dridi
Copy link
Member

@dridi dridi commented Nov 7, 2023

With the introduction of param.set -j performing the regular param.set operation but outputting param.show -j we made it possible for "operators" to atomically set a parameter to a specific value, and know how it would be rendered. This enables safe periodic consistency checks to undo potential manual tweaks.

This change however, left a blind spot for bits parameters, since we either set them to their default value via the "none" or "default" special values, and otherwise with relative values in the form of a series of -xxx or +yyy tokens.

This patch series adds a new "all" value and allows "all" and "none" to appear in the bits list:

param.set foo none,+bar,+baz

It also changes the relative default values of some bits parameters to make them all absolute.

The "default" special value is marked as deprecated in favor of param.reset.

The param.reset command grew a -j option to behave like param.set, for consistency.

Last but not least, it fixes the handling of "none" and "default" that could spuriously leave up to 7 most significant bits raised.

Copy link
Member

@nigoroll nigoroll left a comment

Choose a reason for hiding this comment

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

Makes a lot of sense, I looked through the commits, discussed two details with Dridi, but did not test otherwise. Should be ok (flw). Also good explanation, thank you

The most significant bits of the least significant octet would be
omitted when the number of bits is not a multiple of eight.
As opposed to special values. This enables setting "absolute" values
atomically:

    param.set foo none,+bar,+baz
Better diff with the --ignore-all-space option.
This is primarily for self-documentation purposes, but also to simplify
the bit tweak.
We have had the ability to reset any parameter to its default value for
a while now.
@dridi
Copy link
Member Author

dridi commented Nov 14, 2023

I reworded the first commit message, hence the force-push.

@dridi dridi merged commit 23d62d2 into varnishcache:master Nov 21, 2023
1 check passed
@dridi dridi deleted the absolute_bits branch November 21, 2023 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants