[GitHub] incubator-trafficcontrol issue #289: Added license verifier tool.
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/289 I opened a new PR with the followup to this conversation. This is no longer applicable. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #289: Added license verifier tool.
Github user alficles closed the pull request at: https://github.com/apache/incubator-trafficcontrol/pull/289 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #809: Automatic License Checking with ...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/809 Automatic License Checking with Weasel As we discussed in #289 , this isn't really the right place for a general-purpose license verification tool to live. I've found a new home for it over at https://github.com/Comcast/weasel and just vendored it here. I added a docker task for it that should run fine with `pkg`. It outputs to `dist/weasel.txt`, so we can figure out what failed if we miss a license somewhere. I also fixed some license issues we had hanging around, so that I wouldn't break the build. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol weasel Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/809.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 #809 commit fd61602eb88b6adafb0d89e9284232d5ccbc00c8 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-08-14T23:00:49Z Vendor weasel and add it to the docker build. commit 19e3462dbd5f6387cd28083ad73ec7caf6b8ac71 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-08-14T23:05:16Z Update LICENSE and a few headers to fix weasel complaints. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #786: Fix TO ORT for missing Content-L...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/786#discussion_r132782691 --- Diff: traffic_ops/bin/traffic_ops_ort.pl --- @@ -1623,9 +1623,7 @@ sub check_lwp_response_content_length { my $url = $lwp_response->request->uri; if ( !defined($lwp_response->header('Content-Length')) ) { - ( $log_level >> $panic_level ) && print $log_level_str . " $url did not return a Content-Length header!\n"; - exit; - return 1; + return 0; # Content-Length MAY be omitted per HTTP/1.1 RFC 7230, and in fact MUST NOT be included with a 'Transfer-Encoding: Chunked' header, which MUST be accepted by clients. --- End diff -- Yup. That's great. Or any other fast hash. Security isn't a major issue here, so any reliable hash with low collisions will work. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #786: Fix TO ORT for missing Content-L...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/786#discussion_r132722069 --- Diff: traffic_ops/bin/traffic_ops_ort.pl --- @@ -1623,9 +1623,7 @@ sub check_lwp_response_content_length { my $url = $lwp_response->request->uri; if ( !defined($lwp_response->header('Content-Length')) ) { - ( $log_level >> $panic_level ) && print $log_level_str . " $url did not return a Content-Length header!\n"; - exit; - return 1; + return 0; # Content-Length MAY be omitted per HTTP/1.1 RFC 7230, and in fact MUST NOT be included with a 'Transfer-Encoding: Chunked' header, which MUST be accepted by clients. --- End diff -- Current recommendations are to implement your own `X-` header and use a nice modern hash. Reusing the `Content-MD5` header is not recommended because many clients implemented it (varyingly, leading to trouble) before it was pulled from the spec. I'd either use an `X-Content-SHA256` field or, alternately, a totally separate request that just fetches the hash. (Race conditions about on that approach, though.) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #785: [TC-502] ORT now skips file chec...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/785#discussion_r131968755 --- Diff: traffic_ops/bin/traffic_ops_ort.pl --- @@ -1266,15 +1266,19 @@ sub check_plugins { ( my @parts ) = split( /\@plugin\=/, $liner ); foreach my $i ( 1..$#parts ) { ( my $plugin_name, my $plugin_config_file ) = split( /\@pparam\=/, $parts[$i] ); - if (defined( $plugin_config_file ) ) { + if (defined( $plugin_config_file ) ) {{ --- End diff -- Do we have a standard perl indentation tool? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #785: [TC-502] ORT now skips file chec...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/785#discussion_r131962210 --- Diff: traffic_ops/bin/traffic_ops_ort.pl --- @@ -1266,15 +1266,19 @@ sub check_plugins { ( my @parts ) = split( /\@plugin\=/, $liner ); foreach my $i ( 1..$#parts ) { ( my $plugin_name, my $plugin_config_file ) = split( /\@pparam\=/, $parts[$i] ); - if (defined( $plugin_config_file ) ) { + if (defined( $plugin_config_file ) ) {{ --- End diff -- It's perl. :/ Creates the mandatory block for the early exit. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #785: [TC-502] ORT now skips file chec...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/785 [TC-502] ORT now skips file checks for remap plugin arguments that start with '-'. ORT parses the remap.config file to find dependent files to update. Since parameters to plugins aren't easily distinguished from options to those plugins, this distinguishes based on the first character. It's not ideal, but it works well in practice. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol tc-502-ort-param-args Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/785.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 #785 commit e47b02afa238a22411b7824bc79dcbae110cb718 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-08-08T15:44:08Z [TC-502] ORT now skips file checks for remap plugin arguments that start with '-'. ORT parses the remap.config file to find dependent files to update. Since parameters to plugins aren't easily distinguished from options to those plugins, this distinguishes based on the first character. It's not ideal, but it works well in practice. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #289: Added license verifier tool.
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/289 Patch needs to be reworked. I need to pull out the body of the verifier, but not the supporting script. It's... on the todo list. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r131466564 --- Diff: traffic_ops/install/bin/_postinstall --- @@ -218,7 +234,9 @@ sub generateCdnConf { $#secrets = $cdnConfiguration{keepSecrets} - 1; } } +$cdnConf = setCdnConfGoPort($cdnConf); --- End diff -- What's the default max_db_connections for the Perl version? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r131465943 --- Diff: traffic_ops/install/bin/_postinstall --- @@ -183,6 +183,22 @@ sub generateDbConf { return \%todbconf; } +sub setCdnConfGoPort { +my $cdnConf = shift; +if ( exists $cdnConf->{traffic_ops_golang_port} ) { --- End diff -- I'm confused as to when the port value might be removed. This should always add the port, if it's missing. If the config is regenerated, it might lose the value, but I'm not seeing a code path here that causes the port value to go missing. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r13009 --- Diff: traffic_ops/install/bin/_postinstall --- @@ -183,6 +183,22 @@ sub generateDbConf { return \%todbconf; --- End diff -- Did you intend to strip execute permissions from this? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130711484 --- Diff: traffic_monitor_golang/traffic_monitor/config/config.go --- @@ -21,7 +21,6 @@ package config --- End diff -- Heh, I'm too slow. Apparently a second PR is the right answer. :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130710775 --- Diff: traffic_monitor_golang/traffic_monitor/config/config.go --- @@ -21,7 +21,6 @@ package config --- End diff -- Should the TM changes go in a separate PR on which this one depends, or are you suggesting we branch the log package and keep a separate one for TM and TO? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129993924 --- Diff: traffic_ops/traffic_ops_golang/perlhash.go --- @@ -0,0 +1,249 @@ +package main + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file +* distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. +*/ + +import ( + "fmt" + "strconv" + "strings" + "unicode" +) + +func ParsePerlObj(s string) (map[string]interface{}, error) { + obj, _, err := getObj(s) + return obj, err +} + +func getObj(s string) (map[string]interface{}, string, error) { + obj := map[string]interface{}{} + + s = strings.TrimSpace(s) + if len(s) < 1 || s[0] != '{' { + return obj, "", fmt.Errorf("expected first character '{': %v", s) + } + s = s[1:] // strip opening { + s = strings.TrimSpace(s) + + // read top-level keys + for { + s = stripComment(s) + s = strings.TrimSpace(s) + // s = stripComment(s) + if len(s) > 0 && s[0] == '}' { + return obj, s[1:], nil + } + + key := "" + key, s = getKey(s) + + s = strings.TrimSpace(s) + if len(s) == 0 { + return obj, "", fmt.Errorf("malformed string after key '%v'", key) + } + + err := error(nil) + switch { + case s[0] == '{': + v := map[string]interface{}{} + v, s, err = getObj(s) + if err != nil { + return obj, "", fmt.Errorf("Error getting object value after key %v: %v", key, err) + } + obj[key] = v + case s[0] == '\'': + v := "" + v, s, err = getStr(s) + if err != nil { + return obj, "", fmt.Errorf("Error getting string value after key %v: %v", key, err) + } + obj[key] = v + case unicode.IsDigit(rune(s[0])): --- End diff -- You probably know this, but this whole thing will break if the user is using high unicode digit characters as indexes in their array. (Like `৩`, a Bengali three.) You probably don't care, but I feel I should mention it, in case you're aware of a corner case where it matters. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #729: Traffic Ops Golang Incremental Rewrite ...
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/729 @dewrich `traffic_ops_access.log` is a bit ambiguous, since the whole construct, go+perl, is Traffic Ops. It should be something that clearly indicates it a log for the perl component. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #729: Traffic Ops Golang Incremental Rewrite ...
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/729 From a user's perspective, this creates some extra (as-yet-poorly-documented) obligations for configuration. An RPM upgrade should take care of itself. There should be enough information for the upgrade to perform all the config changes necessary to maintain nearly exactly the same set of functionality users have today. You'll need to allocate a new, high, port, but that's a fairly reasonable requirement, easily met. Notify the user via a message during upgrade and I think it would be ok. I'd love to see this merged; I think it's the right direction. I think we should attend a bit more carefully to the user experience, though. We don't want people afraid to upgrade TO because we break their installs. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #695: TrafficMonitor IPV6 improvements
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/695 This is pretty cool. It looks like there are two separate PRs in here, and the second one could really use a squash. The astats changes can stand on their own, so they should probably get their own PR. And the IPv6 for TM should be squashed. There are some confusing reverts of inadvertent changes that probably shouldn't go in, even as intermediate changesets. It'll be a lot easier to review it that way, I think. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #347: Add global build script.
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/347 Sorry it took so long to circle back to this one, I got caught up with some other stuff. Fixed the license. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #289: Added license verifier tool.
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/289 Yeah. I seriously considered its own repo. But then, if we wanted to run it via CI, and I think we should, we'd have to either include the binary, which is tricky in an Apache project, or vendor the source, which puts it right back here. Any ideas? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111808124 --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go --- @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri } return dispatchMap } + +func acceptsGzip(r *http.Request) bool { + encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests + for _, encodingHeader := range encodingHeaders { + encodingHeader := strings.Replace(encodingHeader, " ", "", -1) + encodings := strings.Split(encodingHeader, ",") + for _, encoding := range encodings { + if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC + return true + } + } + } + return false +} + +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written. +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) { + // TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer. + if len(b) == 0 || !acceptsGzip(r) { + return b, nil + } + w.Header().Set("Content-Encoding", "gzip") + + buf := bytes.Buffer{} + zw := gzip.NewWriter() + + if _, err := zw.Write(b); err != nil { + return nil, fmt.Errorf("gzipping bytes: %v") + } + + if err := zw.Close(); err != nil { + return nil, fmt.Errorf("closing gzip writer: %v") + } + + return buf.Bytes(), nil --- End diff -- I just realized... you have both the original buffer and the compressed one in memory here. You might as well only return the shorter one. That will be the compressed one 99% of the time, but since you have the info, you might as well do the best option. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111799950 --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go --- @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri } return dispatchMap } + +func acceptsGzip(r *http.Request) bool { + encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests + for _, encodingHeader := range encodingHeaders { + encodingHeader := strings.Replace(encodingHeader, " ", "", -1) + encodings := strings.Split(encodingHeader, ",") + for _, encoding := range encodings { + if strings.ToLower(encoding) == "gzip" { // encoding is case-insensitive, per the RFC + return true + } + } + } + return false +} + +// gzipIfAccepts gzips the given bytes, writes a `Content-Encoding: gzip` header to the given writer, and returns the gzipped bytes, if the Request supports GZip (has an Accept-Encoding header). Else, returns the bytes unmodified. Note the given bytes are NOT written to the given writer. It is assumed the bytes may need to pass thru other middleware before being written. +func gzipIfAccepts(r *http.Request, w http.ResponseWriter, b []byte) ([]byte, error) { + // TODO this could be made more efficient by wrapping ResponseWriter with the GzipWriter, and letting callers writer directly to it - but then we'd have to deal with Closing the gzip.Writer. + if len(b) == 0 || !acceptsGzip(r) { --- End diff -- Since we're not using interfaces for composability, we've got the length. Why bother compressing very small strings? Maybe set the minimum compression length at some appropriate non-zero number? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111800861 --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go --- @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc). func WrapBytes(f func() []byte, contentType string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + bytes := f() + bytes, err := gzipIfAccepts(r, w, bytes) + if err != nil { + log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err) + code := http.StatusInternalServerError + w.WriteHeader(code) + if _, err := w.Write([]byte(http.StatusText(code))); err != nil { + log.Warnf("received error writing data request %v: %v\n", r.URL.EscapedPath(), err) + } + return + } + w.Header().Set("Content-Type", contentType) - log.Write(w, f(), r.URL.EscapedPath()) + log.Write(w, bytes, r.URL.EscapedPath()) --- End diff -- `log.Write` is a terrible name. It misleads one into thinking that the purpose of this line is to write to a log at w, not unlike what standard library functions like [io.WriteString](https://golang.org/pkg/io/#WriteString) do. Fixing it is probably slightly out of scope... but I register my objection regardless. :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111799324 --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go --- @@ -157,16 +159,34 @@ func WrapErrCode(errorCount threadsafe.Uint, reqPath string, body []byte, err er // WrapBytes takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc). func WrapBytes(f func() []byte, contentType string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + bytes := f() + bytes, err := gzipIfAccepts(r, w, bytes) + if err != nil { --- End diff -- Shouldn't this error be handled the same as others? I think it should increment errorCount, like all the other routines, right? And if it's doing that, why wouldn't it just be a wrapper around WrapErr, which contrary to the indication one might glean from it's name is just the same as this, but with errors as well. In fact, this function might could be written: return WrapErr( errorCode /* that probably needs to be passed in */, func() ([]byte, err) { return f(), nil }, contentType) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #425: Add Traffic Monitor 2.0 HTTP gzi...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/425#discussion_r111804555 --- Diff: traffic_monitor_golang/traffic_monitor/datareq/datareq.go --- @@ -235,3 +276,39 @@ func addTrailingSlashEndpoints(dispatchMap map[string]http.HandlerFunc) map[stri } return dispatchMap } + +func acceptsGzip(r *http.Request) bool { + encodingHeaders := r.Header["Accept-Encoding"] // headers are case-insensitive, but Go promises to Canonical-Case requests + for _, encodingHeader := range encodingHeaders { + encodingHeader := strings.Replace(encodingHeader, " ", "", -1) --- End diff -- This is incorrect. These headers have optional linear whitespace included, which can be spaces, tabs, or a CRLF, as long as it is followed by a space or a tab. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #397: Fixed incorrect table value in s...
Github user alficles closed the pull request at: https://github.com/apache/incubator-trafficcontrol/pull/397 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #396: Fixed incorrect table value in s...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/396 Fixed incorrect table value in stats endpoint. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol stats-performance-rack-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/396.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 #396 commit ae1338cbef16aee760cab7432ba13c0692454b71 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-22T19:07:24Z Fixed incorrect table value in stats endpoint. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #394: Stats performance 2.0.x
Github user alficles closed the pull request at: https://github.com/apache/incubator-trafficcontrol/pull/394 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #394: Stats performance 2.0.x
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/394 Stats performance 2.0.x Backport of #391 You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol stats-performance-2.0.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/394.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 #394 commit b9cead0fac1e953943b22b7a20af2b3d8f1f05fa Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-21T22:51:33Z Switch stats endpoint to raw sql query for performance. commit 6c6dfb012a5349d6999a1fefe4c54248bf550a07 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-22T16:41:38Z Updated syntax on stats endpoint fetch to use native hash contruction. This improves performance slightly and readability considerably. commit 2f90a92f1ea1b6b41136f23c13c090d0d7209658 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-22T16:48:47Z Fixed indentation in inline SQL. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #391: Switch stats endpoint to raw sql...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/391 Switch stats endpoint to raw sql query for performance. This isn't ideal, but it cuts the the duration of the stats query to a third. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol stats-performance Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/391.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 #391 commit c3641e9b6c1f1589bdba26695db2320b3750bb77 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-21T22:51:33Z Switch stats endpoint to raw sql query for performance. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #347: Add global build script.
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/347 Add global build script. This splits out the somewhat messy docker build command into its own script and adds a global build script. Thereâs still work to be done to clean up docker build scripts, but this provides a basic way to build top to bottom In a single, simple command. This is a step toward addressing TC-180, but falls short of cleaning up everything that requires cleaning up. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol global-build-script Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/347.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 #347 commit 64e18cb57756ed995adc29ab8018abb1d60e73d6 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-10T18:18:57Z Add global build script. This splits out the somewhat messy docker build command into its own script and adds a global build script. Thereâs still work to be done to clean up docker build scripts, but this provides a basic way to build top to bottom In a single, simple command. This is a step toward addressing TC-180, but falls short of cleaning up everything that requires cleaning up. commit 0eb9eddc1b850a8adb934e6b5cc48511fb7ac672 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-03-10T20:20:51Z Updated default build instructions to refer to the build script. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/327 +1. This is good to merge. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/327 Ok. I've tested this. It correctly produces binaries in the `dist` directory, without polluting your environment. For anyone else testing, make sure your docker is on the absolute latest. There's a bug in the penultimate build that is exercised by the `cp -a` in the new script. I'm +1 on this merge, with a few comments: - The `BUILD.md` file doesn't list the new tarball build option. This is a particularly useful option. - The `BUILD.md` identifies the wrong target folder. - The build command is really quite long. This is just an opinion, but we may want to add a tiny script that encodes the commands prescribed by `BUILD.md`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/327 Here's what I do on the ats-rpmbuild side: ``` #!/usr/bin/env bash if [ ! -d /src ]; then echo "Source directory does not exist." exit 1 fi cp -r /src /build cd /build autoreconf -vfi ./configure --prefix=/opt/trafficserver --enable-experimental-plugins --with-max-api-stats=4096 make DESTDIR=/artifacts -j install ``` So the idea is to mount the "live" folder, then copy it to a build folder, build, and copy back the relevant artefacts. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/327 This is a marked improvement, but it introduces a new issue: `make clean`. Since the build is happening directly in the local directories, all the temporary files will be left around. There's two ways around this, either copy the entire build folder into a new directory inside the container and just copy the final binaries out, or introduce a cleaning functionality that will remove the intermediate files. I like the second option better, but the first is significantly easier, I think. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/327 @dangogh @rob05c Yeah, `docker-compose up` should build all the things. As it stands, you'll get either a build failure or, worse, an unlabeled and arbitrary traffic_monitor binary in the output. I would strongly consider deciding which one is "mainstream" and either commenting out the build steps for the other, or reconfiguring the build to put the other's binaries in a different subfolder (which might be tricky). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #289: Added license verifier tool.
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/289 It's worth mentioning... the current master does not pass, because it's licensing is genuinely problematic in ways that are not trivial to correct. So if we decide to add this to CI, we'll probably want to think about how quickly we can remove the problematic library. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #289: Added license verifier tool.
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/289 Added license verifier tool. This is the tool I've been using to grep through the files and categorize them. It's pretty specific to the ASF goals, so it's not a really general purpose tool. It tends significantly toward false positives, which is rather annoying, but reasonably easy to fix. False negatives are simply never caught and can produce considerably more risk. Most of the details are documented in the `README.md` The basic idea is this: Run the tool. For every file to which it objects, either add the appropriate license header, add an appropriate license to the `LICENSE` file, or update the `.dependency_license` file. This routine has nothing to do with Traffic Control and wouldn't really belong in the repo... but it would be very useful to have Continuous Integration run it automatically and for people to be able to run it before summitting PRs. It can find common issues far more quickly than people can manually, which should save contributors' time. I'm definitely open to conversation on this front, including ideas for alternatives. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol auto-license Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/289.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 #289 commit b927192e0560d0dba8ad297328bf14b084f43bd8 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-02-16T22:33:43Z Added license verifier tool. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #285: Updated LICENSE file to contain full te...
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/285 @dangogh Does this look better? The experimental TO got moved to `_golang` and selenium was missing an extension. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #279: [TC-145] Add defines to compile ...
Github user alficles commented on a diff in the pull request: https://github.com/apache/incubator-trafficcontrol/pull/279#discussion_r100902511 --- Diff: traffic_server/plugins/astats_over_http/astats_over_http.c --- @@ -538,9 +538,15 @@ void TSPluginInit(int argc, const char *argv[]) { info.support_email = "jus...@fp-x.com"; astatsLoad = time(NULL); - if (TSPluginRegister() != TS_SUCCESS) { -TSError("Plugin registration failed. \n"); - } + #if (TS_VERSION_NUMBER < 300) + if (TSPluginRegister(TS_SDK_VERSION_2_0, ) != TS_SUCCESS) { + #elif (TS_VERSION_NUMBER < 600) + if (TSPluginRegister(TS_SDK_VERSION_3_0, ) != TS_SUCCESS) { + #else + if (TSPluginRegister() != TS_SUCCESS) { + TSError("Plugin registration failed. \n"); --- End diff -- Is this line intended to be inside the #if? It makes the first two options expand to empty tests, which seems wrong. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #227: Added modernizr to the license f...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/227 Added modernizr to the license file again, since we do use the minifi⦠â¦ed version. This needs to be ported to 1.8.x. @dangogh You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol modernizr-license Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/227.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 #227 commit 062304e432e77cb60a82ee6eed1c5dfce3f752ba Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T23:09:27Z Added modernizr to the license file again, since we do use the minified version. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #224: 1.8.x license fixes
Github user alficles closed the pull request at: https://github.com/apache/incubator-trafficcontrol/pull/224 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #225: Removed a file that merely uses ...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/225 Removed a file that merely uses jmenu from the list of jmenu-licensed⦠⦠things. @dangogh This is the only master-relevant part of #224 . You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol jmenu-license-fixes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/225.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 #225 commit aefa82426b6c1c34db6ed187d5de018396fc67e7 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T22:37:39Z Removed a file that merely uses jmenu from the list of jmenu-licensed things. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #224: 1.8.x license fixes
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/224 1.8.x license fixes These fixes apply only to the 1.8.x branch and should not be moved to master. @dangogh You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol 1.8.x-license-fixes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/224.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 #224 commit 8b8fc2eddce6dcff1b89e2c58984aa8810b821c7 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T22:28:39Z Removed licenses that are not shipped with 1.8. These were inadvertently added when cherry-picking fixes from master. commit fb0b92639af90ce8919f8fe27dafaf387806f840 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T22:37:39Z Removed a file that merely uses jmenu from the list of jmenu-licensed things. commit 687f8c0d5f7f138475f40d27e11e74c2bf50c6f6 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T22:44:06Z Removed Apache header from files to which it does not belong. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #223: Added specific mention of Incons...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/223 Added specific mention of Inconsolata and Late fonts in LICENSE file. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol font-license-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/223.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 #223 commit 4219e5a1075acf14b1a7738dd47848d992808408 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T21:54:20Z Added specific mention of Inconsolata and Late fonts in LICENSE file. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #222: Add link for vendored seelog dep...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/222 Add link for vendored seelog dependency to LICENSE. @dangogh You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol gogit-cleanup3 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/222.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 #222 commit 1ce358140e41d4a76c7c5454c4187a30a24ac752 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T20:57:37Z Add link for vendored seelog dependency to LICENSE. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #221: Remove ssl-bundle license now th...
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/221 Remove ssl-bundle license now that the bundle has been removed. @dangogh You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol ssl-bundle-rm-license Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/221.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 #221 commit 3723016117422d1cfc5b5d9d72557cfc344fc08c Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-24T20:54:39Z Remove ssl-bundle license now that the bundle has been removed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #202: Go/git cleanup
Github user alficles closed the pull request at: https://github.com/apache/incubator-trafficcontrol/pull/202 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol issue #202: Go/git cleanup
Github user alficles commented on the issue: https://github.com/apache/incubator-trafficcontrol/pull/202 This is continued in #220 . I'm closing this one. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #220: Gogit cleanup2
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/220 Gogit cleanup2 This is the same as #202, but with the conflicts fixed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol gogit-cleanup2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/220.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 #220 commit 6c52e39236951fb7eac7e974a5ee77a4f66d9099 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T18:56:29Z Vendored github.com/cihub/seelog. This was previously incorporated as a submodule, though not quite configured correctly. commit 7b0209aa3062c37938957d9dba83110762b5aa07 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T21:20:22Z Replaced vendored traffic_ops with a symlink. Depending on how people configure their gopath, the previous vendored directory could have allowed builds to use an out-dated version of traffic_ops/client. commit 33d4055bec34d7ba2cab809ae1c323fc8896a557 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T23:21:37Z Removed dependency gathering from traffic stats build. All the traffic stats dependencies are and ought to stay vendored, so the build should not go get dependencies. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #202: Go/git cleanup
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/202 Go/git cleanup There were a few issues with the traffic_stats build. This should let the project build again, as well as eliminate the git error that was showing up regarding submodules. Since all the dependencies for traffic stats are currently vendored, I removed the dependency collection. That was masking errors and allowing the project to build, but using the tip of the public repo instead of the expected vendor branch. Other go projects that are fully vendored might want similar treatment at some future time. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol gogit-cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/202.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 #202 commit 81a0defa18c281601402e38e740adca2497b4642 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T18:56:29Z Vendored github.com/cihub/seelog. This was previously incorporated as a submodule, though not quite configured correctly. commit 95f82647e2b8220c6fe4d58a67d1a61208e284a6 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T21:20:22Z Replaced vendored traffic_ops with a symlink. Depending on how people configure their gopath, the previous vendored directory could have allowed builds to use an out-dated version of traffic_ops/client. commit baef17b5439501be9467a0097400c4934d02b8f7 Author: Chris Lemmons <alfic...@gmail.com> Date: 2017-01-19T23:21:37Z Removed dependency gathering from traffic stats build. All the traffic stats dependencies are and ought to stay vendored, so the build should not go get dependencies. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafficcontrol pull request #189: License cleanup part two
GitHub user alficles opened a pull request: https://github.com/apache/incubator-trafficcontrol/pull/189 License cleanup part two This should finish the license cleanup for the rest of master. It should rat clean after this. You can merge this pull request into a Git repository by running: $ git pull https://github.com/alficles/incubator-trafficcontrol license-cleanup-part-two Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-trafficcontrol/pull/189.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 #189 commit 4bc71bc53bd6b9bcbd822db508a7253f44b373c5 Author: Chris Lemmons <chris_lemm...@comcast.com> Date: 2017-01-13T20:56:41Z Removed ATS patch. commit d08451bfa0a055d18322e01f67540aaf99e054fe Author: Chris Lemmons <chris_lemm...@comcast.com> Date: 2017-01-13T20:57:56Z Added Apache license header for a few new files. commit 416b0bcdc7bb6b125b3b1d8fec4138e1c02a7ad2 Author: Chris Lemmons <chris_lemm...@comcast.com> Date: 2017-01-13T20:59:51Z Documented new components in the LICENSE file and updated .rat-excludes appropriately. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---