Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
I'm not sure why you would couple the Golang Microservices with the Traffic
Op Perl Monolith. If anything you should begin a new "rpm" that only contains
the Microservices (which b
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
The reason is to make it as operationally simple as possible. That's the
only reason. We can make more RPMs, microservices, etc after we've moved away
from Perl. I have no objection
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
I agree with simple, but I also don't want to "couple" the two. I don't
find it egregious, to implement them as a "sidecar" rpm. We also might want to
scale them differently becau
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
It should be easy to retire. Once all endpoints are in Golang, simply
delete the Perl from the RPM and Service files.
---
If your project is set up for it, you can reply to this ema
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Also, should `traffic_ops/traffic_ops_golang/auth.go` be in
`traffic_ops/experimental/traffic_ops_golang`? We haven't gotten any buy in
from the community to put that code into `tr
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Right, it _will_ cause build changes, don't merge this until we do get
consensus from the community on the mailing list (I sent an email to `users`
and forwarded to `dev`, "Traffic O
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 infor
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
> upgrade to perform all the config changes necessary to maintain
> allocate a new, high, port
> Notify the user via a message during upgrade
@alficles Agree, will do.
Github user dneuman64 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
It would be nice to see some documentation included with this PR before it
is merged.
Also, I don't see any tests, it would be nice if there were some tests
included as well.
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
So, instead of using your own config, why not just use the existing
`/opt/traffic_ops/app/conf/production/database.conf`. Then you wouldn't have
to prefix each database property wi
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
I rolled an RPM to see how it could work, and I'm not seeing the process
running. Which brought up my next question, where do the logs for
troubleshooting go?
---
If your project
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
If you didn't set up the config, it will fail to start.
Right now, failure to start will be logged to the SystemD service log. I'll
make log locations part of the config, con
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Putting a reminder here: Also need to make setting up the config part of
Postinstall.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
> so then you wouldn't have to touch poinstall
If we want to automatically set up the config, we're going to have to touch
postinstall, if nothing else, to determine the new port
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
@dneuman64 I agree, I'll add docs and tests.
---
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
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
We're also going to need a Perl serialized hash parser in Go, for
`cdn.conf`. It _must_ be modular, so replacing it with JSON is easy once Perl
TO is dead. Tight coupling with Perl h
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Things to do before this can be merged:
- [ ] Configurable Logging (/traffic_monitor_golang/common/log)
- [ ] Tests
- [ ] Documentation
- [ ] Postinstall adding new hig
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Need to add `setcap 'cap_net_bind_service=+ep'
/opt/traffic_ops/traffic_ops_golang` to the RPM, and a dependency on libcap2,
so the Golang service can run as a user and serve 443. Se
Github user elsloo commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
@rob05c awesome, looking forward to seeing how that works, as we might want
to adopt that approach for any/all of our golang components that need to open
ports like this.
---
If yo
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Can we call `perl_access.log` `traffic_ops_access.log`?
---
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 proje
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Looks like the latest changes broke the Mojolicious Cookie
--
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 compon
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
If we plan on making this a "seamless" deployment how does the `to_url` get
configured from traffic_ops_golang.config? That property doesn't exist in the
`cdn.conf`
---
If your
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
It reads the port from `cdn.conf` and sets it to `127.0.0.1:port`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If y
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
As I was performance testing I noticed we don't have any "gates" on the
number of concurrent connections coming into the monitoring service. As
mentioned, the following database po
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
I'll make the config for `MaxOpenConns`. I looked in the Go source, and Go
will block if it hits that limit, rather than asynchronously returning an
error. Which should be fine.
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
My main concern with fine grained control over database connections is that
this service will co-exist with Mojolicious TO database connections, so we have
to make sure they both ca
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Can one of the admins verify this patch?
---
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
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Must be merged after #786 which fixes an ORT bug this exposes.
---
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
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Just ran a JSON comparison tool against the Golang v. Perl response format
and found that they are different. Unfortunately, this can't change until we
rev the API.
Golang
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Per our discussion:
Mojo HTTP Header is sending this back when the Golang proxy does not.
`Cache-Control: no-cache, no-store, max-age=0, must-revalidate`
---
If y
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
This Mojo HTTP Header is also coming back and I don't see it in the Golang
HTTP header
`Connection: keep-alive`
---
If your project is set up for it, you can reply to this
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Test cases failed:
```
--- FAIL: TestGetCDNConf (0.00s)
perlconfig_test.go:84: expected {HTTPPort:443 DBUser: DBPass: DBServer:
DBDB: DBSSL:false TOSecret:walrus TOUR
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/22/
---
If your project is set up for i
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/23/
---
If your project is set up for i
Github user dewrich commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
"test this please"
---
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
Github user asfgit commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/729
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/32/
---
If your project is set up for i
37 matches
Mail list logo
