[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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...

[alficles]

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...

[alficles]

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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r131247154
  
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -218,7 +234,9 @@ sub generateCdnConf {
 $#secrets = $cdnConfiguration{keepSecrets} - 1;
 }
 }
+$cdnConf = setCdnConfGoPort($cdnConf);
--- End diff --

Need another subroutine to account for setting the `max_db_connections` in 
postinstall (or figuring out reasonable defaults) as discussed


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r131246970
  
--- 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 --

This "if statement" needs to be removed because if you run postinstall the 
second time, then `traffic_ops_golang_port` gets removed because there is an 
assumption that the cdn.conf will be always regenerated by postinstall


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130890812
  
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -183,6 +183,22 @@ sub generateDbConf {
 return \%todbconf;
--- End diff --

Fixed.


---
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...

[alficles]

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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130758948
  
--- Diff: docs/source/admin/traffic_ops/configuration.rst ---
@@ -296,5 +296,8 @@ This is a standard kickstart formatted file that the 
generate ISO process uses t
 .. seealso:: For in-depth instructions, please see `Kickstart Installation 
`_
 
 
+Configuring the Go Application
+===
+Traffic Ops is in the process of migrating from Perl to Go, and currently 
runs as two applications. The Go application serves all endpoints which have 
been rewritten in the Go language, and transparently proxies all other requests 
to the old Perl application. Both applications are installed by the RPM, and 
both run as a single service. When the project has fully migrated to Go, the 
Perl application will be removed, and the RPM and service will consist solely 
of the Go application.
 
-
+By default, the postinstall script configures the Go application to behave 
and transparently serve as the old Perl Traffic Ops did in previous versions. 
This includes reading the old ``cdn.conf`` and ``database.conf`` config files, 
and logging to the old ``access.log`` location. However, if you wish to 
customize the Go Traffic Ops application, you can do so by running it with the 
``-oldcfg=false`` argument. By default, it will then look for a config file in 
``/opt/traffic_ops/conf/traffic_ops_golang.json``. The new config file location 
may also be customized via the ``-cfg`` flag. A sample config file is installed 
by the RPM at ``/opt/traffic_ops/conf/traffic_ops_golang.json``. If you wish to 
run the new Go Traffic Ops application as a service with a new config file, the 
``-oldcfg=false`` and  ``-cfg`` flags may be added to the ``start`` function in 
the service file, located by default at ``etc/init.d/traffic_ops``.
--- End diff --

Fixed.


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130754867
  
--- Diff: 
traffic_ops/traffic_ops_golang/vendor/gopkg.in/DATA-DOG/go-sqlmock/LICENSE ---
@@ -0,0 +1,28 @@
+The three clause BSD license (http://en.wikipedia.org/wiki/BSD_licenses)
+
--- End diff --

Done.


---
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...

[alficles]

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...

[alficles]

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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130709846
  
--- Diff: traffic_monitor_golang/traffic_monitor/config/config.go ---
@@ -21,7 +21,6 @@ package config
 
--- End diff --

The config changes have been moved into 
https://github.com/apache/incubator-trafficcontrol/pull/620 which has been 
merged. I'll rebase this to remove them and resolve the conflicts.


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130709645
  
--- Diff: traffic_ops/traffic_ops_golang/monitoring.go ---
@@ -1,90 +1,45 @@
-// Licensed under the Apache License, Version 2.0 (the "License");
--- End diff --

sorry I got lost when I was popping back and forth for this review, I see 
what you mean now



---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130709534
  
--- Diff: traffic_monitor_golang/traffic_monitor/config/config.go ---
@@ -21,7 +21,6 @@ package config
 
--- End diff --

I disagree with TM changes being in the PR because one of the dependencies 
changed, the smaller we can keep this PR the easier it is to merge (and less 
conflicts) we have to deal with


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130630953
  
--- Diff: docs/source/admin/traffic_ops/configuration.rst ---
@@ -296,5 +296,8 @@ This is a standard kickstart formatted file that the 
generate ISO process uses t
 .. seealso:: For in-depth instructions, please see `Kickstart Installation 
`_
 
 
+Configuring the Go Application
+===
+Traffic Ops is in the process of migrating from Perl to Go, and currently 
runs as two applications. The Go application serves all endpoints which have 
been rewritten in the Go language, and transparently proxies all other requests 
to the old Perl application. Both applications are installed by the RPM, and 
both run as a single service. When the project has fully migrated to Go, the 
Perl application will be removed, and the RPM and service will consist solely 
of the Go application.
 
-
+By default, the postinstall script configures the Go application to behave 
and transparently serve as the old Perl Traffic Ops did in previous versions. 
This includes reading the old ``cdn.conf`` and ``database.conf`` config files, 
and logging to the old ``access.log`` location. However, if you wish to 
customize the Go Traffic Ops application, you can do so by running it with the 
``-oldcfg=false`` argument. By default, it will then look for a config file in 
``/opt/traffic_ops/conf/traffic_ops_golang.json``. The new config file location 
may also be customized via the ``-cfg`` flag. A sample config file is installed 
by the RPM at ``/opt/traffic_ops/conf/traffic_ops_golang.json``. If you wish to 
run the new Go Traffic Ops application as a service with a new config file, the 
``-oldcfg=false`` and  ``-cfg`` flags may be added to the ``start`` function in 
the service file, located by default at ``etc/init.d/traffic_ops``.
--- End diff --

Ah, you're right. I meant to rename the file and forgot. I'll change the 
documentation.


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130626620
  
--- Diff: docs/source/admin/traffic_ops/configuration.rst ---
@@ -296,5 +296,8 @@ This is a standard kickstart formatted file that the 
generate ISO process uses t
 .. seealso:: For in-depth instructions, please see `Kickstart Installation 
`_
 
 
+Configuring the Go Application
+===
+Traffic Ops is in the process of migrating from Perl to Go, and currently 
runs as two applications. The Go application serves all endpoints which have 
been rewritten in the Go language, and transparently proxies all other requests 
to the old Perl application. Both applications are installed by the RPM, and 
both run as a single service. When the project has fully migrated to Go, the 
Perl application will be removed, and the RPM and service will consist solely 
of the Go application.
 
-
+By default, the postinstall script configures the Go application to behave 
and transparently serve as the old Perl Traffic Ops did in previous versions. 
This includes reading the old ``cdn.conf`` and ``database.conf`` config files, 
and logging to the old ``access.log`` location. However, if you wish to 
customize the Go Traffic Ops application, you can do so by running it with the 
``-oldcfg=false`` argument. By default, it will then look for a config file in 
``/opt/traffic_ops/conf/traffic_ops_golang.json``. The new config file location 
may also be customized via the ``-cfg`` flag. A sample config file is installed 
by the RPM at ``/opt/traffic_ops/conf/traffic_ops_golang.json``. If you wish to 
run the new Go Traffic Ops application as a service with a new config file, the 
``-oldcfg=false`` and  ``-cfg`` flags may be added to the ``start`` function in 
the service file, located by default at ``etc/init.d/traffic_ops``.
--- End diff --

where do I find this `/opt/traffic_ops/conf/traffic_ops_golang.json` I only 
see `/opt/traffic_ops/conf/traffic_ops_golang.config` in the project, which is 
why I commented on it being documented as `.json`


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r130103970
  
--- 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 --

Eh, I care, but it'd be very expensive time-wise to fix properly. I'm sure 
there are other cases (in fact, we know it doesn't support quoted keys, or 
double-quoted values). But it works for the Perl config as generated by 
Postinstall. And since it's only temporary until Perl goes away, I'm voting we 
don't further delay Step 0 of the migration.

If we hit issues in the future, we can always go back and make the parser 
more robust.


---
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...

[alficles]

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 pull request #729: Traffic Ops Golang Incremental R...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129972669
  
--- Diff: traffic_ops/traffic_ops_golang/monitoring.go ---
@@ -1,90 +1,45 @@
-// Licensed under the Apache License, Version 2.0 (the "License");
--- End diff --

This isn't a monitoring change, this is the `/monitoring` endpoint in 
Traffic Ops.


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129972542
  
--- Diff: traffic_monitor_golang/traffic_monitor/config/config.go ---
@@ -21,7 +21,6 @@ package config
 
--- End diff --

Because the `log` package is used by both, and the `log.InitCfg` was 
changed to take an interface, requiring the Monitor `Config` be given that 
interface's functions.


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129972217
  
--- Diff: traffic_monitor_golang/common/log/log.go ---
@@ -95,6 +97,11 @@ func Eventf(t time.Time, format string, v 
...interface{}) {
Event.Printf("%.3f %s", 
float64(t.Unix())+(float64(t.Nanosecond())/1e9), fmt.Sprintf(format, v...))
 }
 
+// EventfRaw writes to the event log with no prefix.
+func EventfRaw(format string, v ...interface{}) {
--- End diff --

No, it's using the Event logger for `access.log`. Which seemed to make 
sense, accesses are events for this app.


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129972005
  
--- Diff: docs/source/admin/traffic_ops/configuration.rst ---
@@ -296,5 +296,8 @@ This is a standard kickstart formatted file that the 
generate ISO process uses t
 .. seealso:: For in-depth instructions, please see `Kickstart Installation 
`_
 
 
+Configuring the Go Application
+===
+Traffic Ops is in the process of migrating from Perl to Go, and currently 
runs as two applications. The Go application serves all endpoints which have 
been rewritten in the Go language, and transparently proxies all other requests 
to the old Perl application. Both applications are installed by the RPM, and 
both run as a single service. When the project has fully migrated to Go, the 
Perl application will be removed, and the RPM and service will consist solely 
of the Go application.
 
-
+By default, the postinstall script configures the Go application to behave 
and transparently serve as the old Perl Traffic Ops did in previous versions. 
This includes reading the old ``cdn.conf`` and ``database.conf`` config files, 
and logging to the old ``access.log`` location. However, if you wish to 
customize the Go Traffic Ops application, you can do so by running it with the 
``-oldcfg=false`` argument. By default, it will then look for a config file in 
``/opt/traffic_ops/conf/traffic_ops_golang.json``. The new config file location 
may also be customized via the ``-cfg`` flag. A sample config file is installed 
by the RPM at ``/opt/traffic_ops/conf/traffic_ops_golang.json``. If you wish to 
run the new Go Traffic Ops application as a service with a new config file, the 
``-oldcfg=false`` and  ``-cfg`` flags may be added to the ``start`` function in 
the service file, located by default at ``etc/init.d/traffic_ops``.
--- End diff --

`conf/config` seemed redundant, whereas `.json` immediately tells anyone 
looking at it what the format is.


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129960631
  
--- Diff: traffic_ops/traffic_ops_golang/routes.go ---
@@ -0,0 +1,125 @@
+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 (
+   "crypto/tls"
+   "database/sql"
+   "fmt"
+   "net/http"
+   "net/http/httputil"
+   "regexp"
+   "strings"
+)
+
+type ServerData struct {
+   Config
+   DB *sql.DB
+}
+
+type ParamMap map[string]string
+
+type RegexHandlerFunc func(w http.ResponseWriter, r *http.Request, params 
ParamMap)
+
+// getRootHandler returns the / handler for the service, which 
reverse-proxies the old Perl Traffic Ops
+func getRootHandler(d ServerData) http.Handler {
+   // debug
+   tr := {
+   TLSClientConfig: {InsecureSkipVerify: true},
+   }
+   rp := httputil.NewSingleHostReverseProxy(d.TOURL)
+   rp.Transport = tr
+
+   loggingProxyHandler := wrapAccessLog(d.TOSecret, rp)
+   return loggingProxyHandler
+}
+
+// GetRoutes returns the map of regex routes, and a catchall route for 
when no regex matches.
+func GetRoutes(d ServerData) (map[string]RegexHandlerFunc, http.Handler, 
error) {
+   privLevelStmt, err := preparePrivLevelStmt(d.DB)
+   if err != nil {
+   return nil, nil, fmt.Errorf("Error preparing db priv level 
query: ", err)
+   }
+
+   return map[string]RegexHandlerFunc{
+   "api/1.2/cdns/{cdn}/configs/monitoring":  
wrapHeaders(wrapAuth(monitoringHandler(d.DB), d.Insecure, d.TOSecret, 
privLevelStmt, MonitoringPrivLevel)),
--- End diff --

Can we figure out how to make the routes easier to read?  When the routes 
start to grow to what they are today, it'll be difficult to read through them 
in this format.


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129960079
  
--- Diff: traffic_ops/traffic_ops_golang/perlconfig.go ---
@@ -0,0 +1,288 @@
+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 (
+   "encoding/json"
+   "fmt"
+   "io/ioutil"
+   "net/url"
+   "regexp"
+   "strconv"
+   "strings"
+
+   
"github.com/apache/incubator-trafficcontrol/traffic_monitor_golang/common/log"
+)
+
+const OldAccessLogPath = "/var/log/traffic_ops/access.log"
+const NewLogPath = "/var/log/traffic_ops/traffic_ops_golang.log"
+
+func GetPerlConfigs(cdnConfPath string, dbConfPath string) (Config, error) 
{
+   configBytes, err := ioutil.ReadFile(cdnConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading CDN conf '%v': %v", 
cdnConfPath, err)
+   }
+   dbConfBytes, err := ioutil.ReadFile(dbConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading db conf '%v': %v", 
dbConfPath, err)
+   }
+   return getPerlConfigsFromStrs(string(configBytes), string(dbConfBytes))
+}
+
+func getPerlConfigsFromStrs(cdnConfBytes string, dbConfBytes string) 
(Config, error) {
+   cfg, err := getCDNConf(cdnConfBytes)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing CDN conf '%v': %v", 
cdnConfBytes, err)
+   }
+
+   dbconf, err := getDbConf(string(dbConfBytes))
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing db conf '%v': %v", 
dbConfBytes, err)
+   }
+   cfg.DBUser = dbconf.User
+   cfg.DBPass = dbconf.Password
+   cfg.DBServer = dbconf.Hostname
+   cfg.DBDB = dbconf.DBName
+   cfg.DBSSL = false // TODO fix
+   if dbconf.Port != "" {
+   cfg.DBServer += ":" + dbconf.Port
+   }
+
+   cfg.LogLocationInfo = OldAccessLogPath
+   cfg.LogLocationError = NewLogPath
+   cfg.LogLocationWarning = NewLogPath
+   cfg.LogLocationEvent = NewLogPath
+   cfg.LogLocationDebug = log.LogLocationNull
+
+   return cfg, nil
+}
+
+func getCDNConf(s string) (Config, error) {
+   cfg := Config{}
+   obj, err := ParsePerlObj(s)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing Perl object: %v", err)
+   }
+
+   if cfg.HTTPPort, err = getPort(obj); err != nil {
+   return Config{}, err
+   }
+
+   if cfg.TOSecret, err = getSecret(obj); err != nil {
+   return Config{}, err
+   }
+
+   oldPort, err := getOldPort(obj)
+   if err != nil {
+   return Config{}, err
+   }
+   cfg.TOURLStr = "https://127.0.0.1:; + oldPort
+   if cfg.TOURL, err = url.Parse(cfg.TOURLStr); err != nil {
+   return Config{}, fmt.Errorf("Invalid Traffic Ops URL '%v': 
err", cfg.TOURL, err)
+   }
+
+   cfg.CertPath, err = getConfigCert(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   cfg.KeyPath, err = getConfigKey(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   return cfg, nil
+}
+
+func getPort(obj map[string]interface{}) (string, error) {
+   portStrI, ok := obj["traffic_ops_golang_port"]
+   if !ok {
+   return "", fmt.Errorf("missing traffic_ops_golang_port key")
+   }
+   portStr, ok := portStrI.(string)
+   if !ok {
--- End diff --

Was wondering why the decision was made to use a string for a port.  Not 
what I usually see in config files


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129959254
  
--- Diff: traffic_ops/traffic_ops_golang/monitoring.go ---
@@ -1,90 +1,45 @@
-// Licensed under the Apache License, Version 2.0 (the "License");
--- End diff --

Same comment as above, why are monitoring changes in this PR?


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129958365
  
--- Diff: traffic_monitor_golang/traffic_monitor/config/config.go ---
@@ -21,7 +21,6 @@ package config
 
--- End diff --

Why are Traffic Monitor changes in this PR?  


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129481493
  
--- Diff: traffic_ops/traffic_ops_golang/config.go ---
@@ -0,0 +1,127 @@
+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 (
+   "encoding/json"
+   "fmt"
+   "io/ioutil"
+   "net/url"
+
+   
"github.com/apache/incubator-trafficcontrol/traffic_monitor_golang/common/log"
+)
+
+type Config struct {
+   HTTPPort   string   `json:"port"`
+   DBUser string   `json:"db_user"`
+   DBPass string   `json:"db_pass"`
+   DBServer   string   `json:"db_server"`
+   DBDB   string   `json:"db_name"`
+   DBSSL  bool `json:"db_ssl"`
+   TOSecret   string   `json:"to_secret"`
+   TOURLStr   string   `json:"to_url"`
+   TOURL  *url.URL `json:"-"`
+   NoAuth bool `json:"no_auth"`
+   CertPath   string   `json:"cert_path"`
+   KeyPathstring   `json:"key_path"`
+   LogLocationError   string   `json:"log_location_error"`
+   LogLocationWarning string   `json:"log_location_warning"`
+   LogLocationInfostring   `json:"log_location_info"`
+   LogLocationDebug   string   `json:"log_location_debug"`
+   LogLocationEvent   string   `json:"log_location_event"`
+}
+
+func (c Config) Error() log.LogLocation   { return 
log.LogLocation(c.LogLocationError) }
+func (c Config) Warning() log.LogLocation { return 
log.LogLocation(c.LogLocationWarning) }
+func (c Config) Info() log.LogLocation{ return 
log.LogLocation(c.LogLocationInfo) }
+func (c Config) Debug() log.LogLocation   { return 
log.LogLocation(c.LogLocationDebug) }
+func (c Config) Event() log.LogLocation   { return 
log.LogLocation(c.LogLocationEvent) }
+
+func LoadConfig(fileName string) (Config, error) {
+   if fileName == "" {
+   return Config{}, fmt.Errorf("no filename")
+   }
+
+   configBytes, err := ioutil.ReadFile(fileName)
+   if err != nil {
+   return Config{}, err
+   }
+
+   cfg := Config{}
+   if err := json.Unmarshal(configBytes, ); err != nil {
+   return Config{}, err
+   }
+
+   if cfg, err = ParseConfig(cfg); err != nil {
+   return Config{}, err
+   }
+
+   return cfg, nil
+}
+
+// ParseConfig validates required fields, and parses non-JSON types
+func ParseConfig(cfg Config) (Config, error) {
--- End diff --

Done.


---
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...

[rob05c]

Github user rob05c commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129080524
  
--- Diff: traffic_ops/traffic_ops_golang/perlconfig.go ---
@@ -0,0 +1,288 @@
+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 (
+   "encoding/json"
+   "fmt"
+   "io/ioutil"
+   "net/url"
+   "regexp"
+   "strconv"
+   "strings"
+
+   
"github.com/apache/incubator-trafficcontrol/traffic_monitor_golang/common/log"
+)
+
+const OldAccessLogPath = "/var/log/traffic_ops/access.log"
+const NewLogPath = "/var/log/traffic_ops/traffic_ops_golang.log"
+
+func GetPerlConfigs(cdnConfPath string, dbConfPath string) (Config, error) 
{
+   configBytes, err := ioutil.ReadFile(cdnConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading CDN conf '%v': %v", 
cdnConfPath, err)
+   }
+   dbConfBytes, err := ioutil.ReadFile(dbConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading db conf '%v': %v", 
dbConfPath, err)
+   }
+   return getPerlConfigsFromStrs(string(configBytes), string(dbConfBytes))
+}
+
+func getPerlConfigsFromStrs(cdnConfBytes string, dbConfBytes string) 
(Config, error) {
+   cfg, err := getCDNConf(cdnConfBytes)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing CDN conf '%v': %v", 
cdnConfBytes, err)
+   }
+
+   dbconf, err := getDbConf(string(dbConfBytes))
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing db conf '%v': %v", 
dbConfBytes, err)
+   }
+   cfg.DBUser = dbconf.User
+   cfg.DBPass = dbconf.Password
+   cfg.DBServer = dbconf.Hostname
+   cfg.DBDB = dbconf.DBName
+   cfg.DBSSL = false // TODO fix
+   if dbconf.Port != "" {
+   cfg.DBServer += ":" + dbconf.Port
+   }
+
+   cfg.LogLocationInfo = OldAccessLogPath
+   cfg.LogLocationError = NewLogPath
+   cfg.LogLocationWarning = NewLogPath
+   cfg.LogLocationEvent = NewLogPath
+   cfg.LogLocationDebug = log.LogLocationNull
+
+   return cfg, nil
+}
+
+func getCDNConf(s string) (Config, error) {
+   cfg := Config{}
+   obj, err := ParsePerlObj(s)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing Perl object: %v", err)
+   }
+
+   if cfg.HTTPPort, err = getPort(obj); err != nil {
+   return Config{}, err
+   }
+
+   if cfg.TOSecret, err = getSecret(obj); err != nil {
+   return Config{}, err
+   }
+
+   oldPort, err := getOldPort(obj)
+   if err != nil {
+   return Config{}, err
+   }
+   cfg.TOURLStr = "https://127.0.0.1:; + oldPort
+   if cfg.TOURL, err = url.Parse(cfg.TOURLStr); err != nil {
+   return Config{}, fmt.Errorf("Invalid Traffic Ops URL '%v': 
err", cfg.TOURL, err)
+   }
+
+   cfg.CertPath, err = getConfigCert(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   cfg.KeyPath, err = getConfigKey(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   return cfg, nil
+}
+
+func getPort(obj map[string]interface{}) (string, error) {
+   portStrI, ok := obj["traffic_ops_golang_port"]
+   if !ok {
+   return "", fmt.Errorf("missing traffic_ops_golang_port key")
+   }
+   portStr, ok := portStrI.(string)
+   if !ok {
--- End diff --

It's a string just because it was easier to work with in the code, e.g. the 
Go HTTP server takes a string. I can make it an `int` or `uint` if you want. 
I'd rather not `uint16` though, it's unusual, and even if performance mattered 
it's not any faster on a 64-bit processor.


---
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 

[GitHub] incubator-trafficcontrol pull request #729: Traffic Ops Golang Incremental R...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129072786
  
--- Diff: traffic_ops/traffic_ops_golang/perlconfig.go ---
@@ -0,0 +1,288 @@
+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 (
+   "encoding/json"
+   "fmt"
+   "io/ioutil"
+   "net/url"
+   "regexp"
+   "strconv"
+   "strings"
+
+   
"github.com/apache/incubator-trafficcontrol/traffic_monitor_golang/common/log"
+)
+
+const OldAccessLogPath = "/var/log/traffic_ops/access.log"
+const NewLogPath = "/var/log/traffic_ops/traffic_ops_golang.log"
+
+func GetPerlConfigs(cdnConfPath string, dbConfPath string) (Config, error) 
{
+   configBytes, err := ioutil.ReadFile(cdnConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading CDN conf '%v': %v", 
cdnConfPath, err)
+   }
+   dbConfBytes, err := ioutil.ReadFile(dbConfPath)
+   if err != nil {
+   return Config{}, fmt.Errorf("reading db conf '%v': %v", 
dbConfPath, err)
+   }
+   return getPerlConfigsFromStrs(string(configBytes), string(dbConfBytes))
+}
+
+func getPerlConfigsFromStrs(cdnConfBytes string, dbConfBytes string) 
(Config, error) {
+   cfg, err := getCDNConf(cdnConfBytes)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing CDN conf '%v': %v", 
cdnConfBytes, err)
+   }
+
+   dbconf, err := getDbConf(string(dbConfBytes))
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing db conf '%v': %v", 
dbConfBytes, err)
+   }
+   cfg.DBUser = dbconf.User
+   cfg.DBPass = dbconf.Password
+   cfg.DBServer = dbconf.Hostname
+   cfg.DBDB = dbconf.DBName
+   cfg.DBSSL = false // TODO fix
+   if dbconf.Port != "" {
+   cfg.DBServer += ":" + dbconf.Port
+   }
+
+   cfg.LogLocationInfo = OldAccessLogPath
+   cfg.LogLocationError = NewLogPath
+   cfg.LogLocationWarning = NewLogPath
+   cfg.LogLocationEvent = NewLogPath
+   cfg.LogLocationDebug = log.LogLocationNull
+
+   return cfg, nil
+}
+
+func getCDNConf(s string) (Config, error) {
+   cfg := Config{}
+   obj, err := ParsePerlObj(s)
+   if err != nil {
+   return Config{}, fmt.Errorf("parsing Perl object: %v", err)
+   }
+
+   if cfg.HTTPPort, err = getPort(obj); err != nil {
+   return Config{}, err
+   }
+
+   if cfg.TOSecret, err = getSecret(obj); err != nil {
+   return Config{}, err
+   }
+
+   oldPort, err := getOldPort(obj)
+   if err != nil {
+   return Config{}, err
+   }
+   cfg.TOURLStr = "https://127.0.0.1:; + oldPort
+   if cfg.TOURL, err = url.Parse(cfg.TOURLStr); err != nil {
+   return Config{}, fmt.Errorf("Invalid Traffic Ops URL '%v': 
err", cfg.TOURL, err)
+   }
+
+   cfg.CertPath, err = getConfigCert(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   cfg.KeyPath, err = getConfigKey(obj)
+   if err != nil {
+   return Config{}, err
+   }
+
+   return cfg, nil
+}
+
+func getPort(obj map[string]interface{}) (string, error) {
+   portStrI, ok := obj["traffic_ops_golang_port"]
+   if !ok {
+   return "", fmt.Errorf("missing traffic_ops_golang_port key")
+   }
+   portStr, ok := portStrI.(string)
+   if !ok {
--- End diff --

Shouldn't the port be defined as an "integer"?


---
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...

[dewrich]

Github user dewrich commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/729#discussion_r129066134
  
--- Diff: traffic_ops/traffic_ops_golang/config.go ---
@@ -0,0 +1,127 @@
+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 (
+   "encoding/json"
+   "fmt"
+   "io/ioutil"
+   "net/url"
+
+   
"github.com/apache/incubator-trafficcontrol/traffic_monitor_golang/common/log"
+)
+
+type Config struct {
+   HTTPPort   string   `json:"port"`
+   DBUser string   `json:"db_user"`
+   DBPass string   `json:"db_pass"`
+   DBServer   string   `json:"db_server"`
+   DBDB   string   `json:"db_name"`
+   DBSSL  bool `json:"db_ssl"`
+   TOSecret   string   `json:"to_secret"`
+   TOURLStr   string   `json:"to_url"`
+   TOURL  *url.URL `json:"-"`
+   NoAuth bool `json:"no_auth"`
+   CertPath   string   `json:"cert_path"`
+   KeyPathstring   `json:"key_path"`
+   LogLocationError   string   `json:"log_location_error"`
+   LogLocationWarning string   `json:"log_location_warning"`
+   LogLocationInfostring   `json:"log_location_info"`
+   LogLocationDebug   string   `json:"log_location_debug"`
+   LogLocationEvent   string   `json:"log_location_event"`
+}
+
+func (c Config) Error() log.LogLocation   { return 
log.LogLocation(c.LogLocationError) }
+func (c Config) Warning() log.LogLocation { return 
log.LogLocation(c.LogLocationWarning) }
+func (c Config) Info() log.LogLocation{ return 
log.LogLocation(c.LogLocationInfo) }
+func (c Config) Debug() log.LogLocation   { return 
log.LogLocation(c.LogLocationDebug) }
+func (c Config) Event() log.LogLocation   { return 
log.LogLocation(c.LogLocationEvent) }
+
+func LoadConfig(fileName string) (Config, error) {
+   if fileName == "" {
+   return Config{}, fmt.Errorf("no filename")
+   }
+
+   configBytes, err := ioutil.ReadFile(fileName)
+   if err != nil {
+   return Config{}, err
+   }
+
+   cfg := Config{}
+   if err := json.Unmarshal(configBytes, ); err != nil {
+   return Config{}, err
+   }
+
+   if cfg, err = ParseConfig(cfg); err != nil {
+   return Config{}, err
+   }
+
+   return cfg, nil
+}
+
+// ParseConfig validates required fields, and parses non-JSON types
+func ParseConfig(cfg Config) (Config, error) {
--- End diff --

Feedback I got from the Traffic Logs Generator was each config error should 
be listed at once.  It's painful for the developer to fix the config problems 
one by one when they could just look at an error 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...

[rob05c]

GitHub user rob05c opened a pull request:

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

Traffic Ops Golang Incremental Rewrite App

This adds an app, which serves Traffic Ops endpoints as they're written 
(currently, just monitoring.json), and reverse-proxies everything else to the 
old Perl Traffic Ops.

Includes RPM and Service files, to deploy it alongside the old TO.

This can be trivially deployed alongside the old TO with 2 simple config 
deployment (Puppet) changes: changing the port on the old TO, and adding the 
small config for the new TO.

**Do not** merge this without consensus on the mailing list. It modifies 
the RPM and _will_ affect deployment.


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

$ git pull https://github.com/rob05c/incubator-trafficcontrol 
to-gomonitoring

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

https://github.com/apache/incubator-trafficcontrol/pull/729.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 #729


commit 07606b1e7ef018d2f7b0c2d68f475864c6d2e29f
Author: Robert Butts 
Date:   2017-07-09T03:02:56Z

Add experimental Go TO proxying old Perl app

commit c75817be2eb8977ffd7573b67d0e060e5f38ee03
Author: Robert Butts 
Date:   2017-07-09T15:22:28Z

Move TO Golang microservice out of experimental

commit f374350a3e767fbb95239da06258436e04a0b1e2
Author: Robert Butts 
Date:   2017-07-09T15:23:49Z

Add traffic_ops_golang to RPM, service




---
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.
---