You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "ezelkow1 (via GitHub)" <gi...@apache.org> on 2024/12/06 21:14:04 UTC

[PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

ezelkow1 opened a new pull request, #11902:
URL: https://github.com/apache/trafficserver/pull/11902

   Fixes #11901


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2954461342

   This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2524219915

   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.
   https://github.com/apache/trafficserver/blob/dc59c4c1fc8243f60601127442ea6835c434ffdd/tools/records/convert2yaml.py#L48-L49
   
   I think the best way is to support the both names (`.value` and `.enabled`) for now, and introduce new name (`.mode`) to address the inconsistency between session_cache and origin_session_cache.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2529754070

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2605322141

   We can introduce `mode` later if we want. Sooner would be better but I think 10.1 is reasonable (and we should deprecate the current ones). For now, we should address the inconsistency between the documentation and the implementation.
   
   If we support the both names (for this particular setting), which one wins if the both `enabled` and `value` exist and have different values? Should it be an error?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "ezelkow1 (via GitHub)" <gi...@apache.org>.
ezelkow1 commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2524223990

   yea I did notice that the upgrading to 10 doc does mention some of these. So there is a reference there for previous users:
   https://docs.trafficserver.apache.org/en/10.0.x/release-notes/upgrading.en.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #11902:
URL: https://github.com/apache/trafficserver/pull/11902#issuecomment-2535288255

   [Implementation](https://github.com/apache/trafficserver/blob/master/src/records/RecordsConfig.cc#L1171C18-L1171C54) seems fine, at least it follows what was the logic when this fields were [renamed](https://github.com/apache/trafficserver/pull/9264):
   
   ```
   Naming Changes:
   
   All these renamed records are subject to the following reasoning:
   
   If a value is meant to be used as a toggle on/off, I’ve used the name enabled.
   If a value is meant to be used as an enumeration, I’ve used the name value
   ```
   
   So `proxy.config.ssl.origin_session_cache.enabled` and `proxy.config.ssl.session_cache.value` were renamed base on the above but documentation wasn't properly updated.
   
   I do agree that `mode` is more appropriate than `value`, but seems late to make this ammend.
   
   
   [This](https://github.com/apache/trafficserver/blob/master/doc/release-notes/upgrading.en.rst?plain=1#L175) needs to be updated too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Update ssl.session_cache.enabled to be session_cache.value [trafficserver]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #11902: Update ssl.session_cache.enabled to be session_cache.value
URL: https://github.com/apache/trafficserver/pull/11902


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org