-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update ssl.session_cache.enabled to be session_cache.value #11902
base: master
Are you sure you want to change the base?
Conversation
I thought the implementation should be changed instead because the documentation is what users see and new users who didn't use ATS 9 would be required to update their settings. But, unfortunately, convert2yaml.py disagrees. trafficserver/tools/records/convert2yaml.py Lines 48 to 49 in dc59c4c
I think the best way is to support the both names ( |
yea I did notice that the upgrading to 10 doc does mention some of these. So there is a reference there for previous users: Also having the old one there is apparently required for the upgrading document to be buildable? This seems like an odd dependency on an old name: Warning, treated as error: |
It looks like there are other configuration options named proxy..enabled and this is the only one with proxy..value. It would be great if this was consistent with other settings. @maskit suggested supporting both configuration option names for now. I would lean towards this as a solution too. |
There was a reason why we have In this case Implementation seems fine, at least it follows what was the logic when this fields were renamed:
So I do agree that upgrading.en.rst needs to be updated too. |
We can introduce If we support the both names (for this particular setting), which one wins if the both |
Fixes #11901