You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by elsloo <gi...@git.apache.org> on 2017/01/09 21:07:24 UTC

[GitHub] incubator-trafficcontrol pull request #172: [TC-91] TM2 - Add support for pa...

GitHub user elsloo opened a pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/172

    [TC-91] TM2 - Add support for path based filtering to applicable lega\u2026

    \u2026cy APIs.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/elsloo/incubator-trafficcontrol 2.0.x_legacy_api_add_filtering

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafficcontrol/pull/172.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #172
    
----
commit f73437d317cfc5eb3f92585145711f559e4fcd67
Author: Jeff Elsloo <je...@cable.comcast.com>
Date:   2017-01-09T21:05:39Z

    [TC-91] TM2 - Add support for path based filtering to applicable legacy APIs.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #172: [TC-91] TM2 - Add support for pa...

Posted by elsloo <gi...@git.apache.org>.
Github user elsloo commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/172#discussion_r95268661
  
    --- Diff: traffic_monitor/experimental/traffic_monitor/manager/datarequest.go ---
    @@ -198,6 +204,16 @@ func NewCacheStatFilter(params url.Values, cacheTypes map[enum.CacheName]enum.Ca
     	}, nil
     }
     
    +// This is the "spirit" of how TM1.0 works; hack to extract a path argument to filter data (/publish/SomeEndpoint/:argument).
    +func getPathArgument(path string) string {
    +	pathParts := strings.Split(path, "/")
    +	if len(pathParts) == 4 {
    --- End diff --
    
    I considered making it `>=` but decided against it. I don't have strong feelings so I'll go ahead and make that change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #172: [TC-91] TM2 - Add support for pa...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/172#discussion_r95404995
  
    --- Diff: traffic_monitor/experimental/traffic_monitor/manager/datarequest.go ---
    @@ -198,6 +204,16 @@ func NewCacheStatFilter(params url.Values, cacheTypes map[enum.CacheName]enum.Ca
     	}, nil
     }
     
    +// This is the "spirit" of how TM1.0 works; hack to extract a path argument to filter data (/publish/SomeEndpoint/:argument).
    +func getPathArgument(path string) string {
    +	pathParts := strings.Split(path, "/")
    +	if len(pathParts) == 4 {
    --- End diff --
    
    I don't have strong feelings on `>=` vs `==`, but I do have strong feelings that _if_ we're rejecting extra path params, we should definitely log and/or return an error explaining why the filter was rejected; ideally both.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #172: [TC-91] TM2 - Add support for pa...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafficcontrol/pull/172


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #172: [TC-91] TM2 - Add support for pa...

Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/172#discussion_r95263802
  
    --- Diff: traffic_monitor/experimental/traffic_monitor/manager/datarequest.go ---
    @@ -198,6 +204,16 @@ func NewCacheStatFilter(params url.Values, cacheTypes map[enum.CacheName]enum.Ca
     	}, nil
     }
     
    +// This is the "spirit" of how TM1.0 works; hack to extract a path argument to filter data (/publish/SomeEndpoint/:argument).
    +func getPathArgument(path string) string {
    +	pathParts := strings.Split(path, "/")
    +	if len(pathParts) == 4 {
    --- End diff --
    
    Should this be `len(pathParts) >= 4`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---