You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "zwoop (via GitHub)" <gi...@apache.org> on 2023/04/07 19:28:24 UTC
[GitHub] [trafficserver] zwoop opened a new pull request, #9590: Adds a new hash_url directive to strategies
zwoop opened a new pull request, #9590:
URL: https://github.com/apache/trafficserver/pull/9590
In addition, this deprecates the old hash_key: cache_key directive. Not only is it replaces with this feature, the existing implementation is confusing because it's not the cache-key that's hashed on. It's the URL as set with TSHttpTxnParentSelectionUrlSet().
--
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
[GitHub] [trafficserver] jrushford commented on pull request #9590: Adds a new hash_url directive to strategies
Posted by "jrushford (via GitHub)" <gi...@apache.org>.
jrushford commented on PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590#issuecomment-1502674497
> > @zwoop, I don't see any unit tests for hash_url. See proxy/http/remap/unit-tests/ the yaml file there are setup with differing strategies such as one using cache_key and the tests check it. Otherwise this looks good.
>
>
>
> I modified a few of the autests to use the new directive. Hopefully it still passes :).
Ok
--
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
[GitHub] [trafficserver] zwoop commented on pull request #9590: Adds a new hash_url directive to strategies
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590#issuecomment-1502048633
[approve ci-clang-analyzer]
--
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
[GitHub] [trafficserver] zwoop merged pull request #9590: Adds a new hash_url directive to strategies
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop merged PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590
--
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
[GitHub] [trafficserver] ezelkow1 commented on a diff in pull request #9590: Adds a new hash_url directive to strategies
Posted by "ezelkow1 (via GitHub)" <gi...@apache.org>.
ezelkow1 commented on code in PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590#discussion_r1162133468
##########
doc/admin-guide/files/strategies.yaml.en.rst:
##########
@@ -182,9 +182,18 @@ Each **strategy** in the list may using the following parameters:
#. **path**: (**default**) Creates a hash over the path portion of the request URL.
#. **path+query**: Same as **path** but adds the **query string** in the request URL.
#. **path+fragment**: Same as **path** but adds the fragment portion of the URL.
- #. **cache_key**: Uses the hash key from the **cachekey** plugin. defaults to **path** if the **cachekey** plugin is not configured on the **remap**.
+ #. **cache_key**: Deprecated, use the URL designation instead.
Review Comment:
Since we are deprecating/changing how some of this works it would be good if there was a deprecation note on what settings to use so that this can act how it used to pre-changes. Just helps with transition
--
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
[GitHub] [trafficserver] zwoop commented on a diff in pull request #9590: Adds a new hash_url directive to strategies
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on code in PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590#discussion_r1162142796
##########
doc/admin-guide/files/strategies.yaml.en.rst:
##########
@@ -182,9 +182,18 @@ Each **strategy** in the list may using the following parameters:
#. **path**: (**default**) Creates a hash over the path portion of the request URL.
#. **path+query**: Same as **path** but adds the **query string** in the request URL.
#. **path+fragment**: Same as **path** but adds the fragment portion of the URL.
- #. **cache_key**: Uses the hash key from the **cachekey** plugin. defaults to **path** if the **cachekey** plugin is not configured on the **remap**.
+ #. **cache_key**: Deprecated, use the URL designation instead.
Review Comment:
Added.
--
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
[GitHub] [trafficserver] zwoop commented on pull request #9590: Adds a new hash_url directive to strategies
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #9590:
URL: https://github.com/apache/trafficserver/pull/9590#issuecomment-1502424820
> @zwoop, I don't see any unit tests for hash_url. See proxy/http/remap/unit-tests/ the yaml file there are setup with differing strategies such as one using cache_key and the tests check it. Otherwise this looks good.
I modified a few of the autests to use the new directive. Hopefully it still passes :).
--
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