You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by Rawlin Peters <ra...@gmail.com> on 2018/06/20 15:54:47 UTC

Removing dead Perl code

Hey Traffic Controllers,

In order to accelerate our progress toward using and developing
traffic_ops_golang as a community, I'd like to propose that we remove
all of the dead Perl Traffic Ops API code in the repo. Many endpoints
have been rewritten in Go at this point, and by keeping the obsolete
Perl endpoints in the repo we're not making it clear that new
enhancements have to be made in traffic_ops_golang and Traffic Portal
in order to actually work and survive long-term (as opposed to the
legacy Perl Traffic Ops API and UI which is planned to be deprecated
and removed in the near future).

Right now the only thing keeping the dead Perl endpoints from being
deleted is the Perl test framework that depends on them. Unfortunately
the tests are all caught in a spider web of interdependency, so we
can't simply remove tests for APIs that have been rewritten in Go
without breaking other tests. However, we think there are a few ways
we should be able to accomplish this:

1. Make the Perl integration tests actually make HTTP calls which can
then be handled by traffic_ops_golang, rather than calling the Perl
API router directly.
2. Remove test interdependencies by mocking out the test data.
3. Rewrite all the Perl tests in the go API test framework and remove
the Perl tests.

We are also open to other suggestions that allow us to remove dead Perl code :).

Please +1 or -1 if you agree or disagree; all feedback is welcome.

- Rawlin

Re: Removing dead Perl code

Posted by Jeremy Mitchell <mi...@gmail.com>.
it sounds great in theory but may be easier said than done. at the very
least the route from TrafficOpsRoutes.pm should be deleted when rewritten
in Go. should the handler and all the functions used by the handler be
deleted as well? like dan said, that may be a messy effort with a high
level of risk as our perl tests are crumbling so you'll really have no idea
what you potentially broke.

just my 2 cents. of course i'm in favor of removing dead code whenever
possible (depending on the effort / risk level).

and yes, i strongly agree with Dave that the rewrite of the api should be a
good opportunity to write valuable tests. I look at it this way. If you
want something to continue working, write tests. if you don't care, don't
write tests. :)

jeremy

On Wed, Jun 20, 2018 at 3:34 PM, Dave Neuman <ne...@apache.org> wrote:

> +1 on removing dead code.
> I think I am mostly in agreement with Rob, I think we should remove the
> dead code and delete the Perl tests that break as a result.  In my opinion
> the tests aren't very useful as they are pretty fagile (hard-coded array
> numbers) and aren't doing that much validation anyway.  If we can come up
> with a reasonable way to replace a perl test with a test in golang that
> exercises the Perl API then we should attempt to do it.  As we re-write
> endpoints in go, we should make sure that we write useful tests using the
> testing framework including writing whatever needs to be written in the
> golang client to support the test.
>
> Thanks,
> Dave
>
>
> On Wed, Jun 20, 2018 at 10:50 AM Dan Kirkwood <da...@gmail.com> wrote:
>
> > I'm all for #3.   In fact,  the `traffic_ops/testing/api` framework that
> > Dewayne has developed encourages testing from the HTTP calls (which you
> > suggest in #1).  That covers the endpoint no matter how it's implemented.
> >
> > Removing dead Perl code can be tricky,  since the interdependencies are
> not
> > always obvious, and can lead to unforeseen missing functionality.  TO
> uses
> > dynamic addition of methods to objects.  The Mojolicious framework, in
> > fact, encourages this.   I'm in favor of getting better testing in place
> > that covers existing endpoints no matter where they fall.  As the Go code
> > progresses,  remove the endpoints from Perl (but not the supporting code)
> > and remove tests as they are added on the Go side.   Remove the whole
> Perl
> > code once the Go code is complete.
> >
> > -dan
> >
> > On Wed, Jun 20, 2018 at 10:18 AM Jeremy Mitchell <mi...@gmail.com>
> > wrote:
> >
> > > #1 is pretty hard / not feasible. I believe Dewayne already looked into
> > > that
> > > #2 sounds hard and probably work that would just be wasted imo
> > > #3 i like this idea. let's pretend we have 100 API tests in perl.
> rewrite
> > > all 100 to Go, delete the perl tests entirely. this is also a good idea
> > > because it provides somewhat of a safety net for the perl to Go api
> > rewrite
> > > which i don't think exists currently.
> > >
> > > Jeremy
> > >
> > > On Wed, Jun 20, 2018 at 9:54 AM, Rawlin Peters <
> rawlin.peters@gmail.com>
> > > wrote:
> > >
> > > > Hey Traffic Controllers,
> > > >
> > > > In order to accelerate our progress toward using and developing
> > > > traffic_ops_golang as a community, I'd like to propose that we remove
> > > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > > > have been rewritten in Go at this point, and by keeping the obsolete
> > > > Perl endpoints in the repo we're not making it clear that new
> > > > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > > > in order to actually work and survive long-term (as opposed to the
> > > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > > > and removed in the near future).
> > > >
> > > > Right now the only thing keeping the dead Perl endpoints from being
> > > > deleted is the Perl test framework that depends on them.
> Unfortunately
> > > > the tests are all caught in a spider web of interdependency, so we
> > > > can't simply remove tests for APIs that have been rewritten in Go
> > > > without breaking other tests. However, we think there are a few ways
> > > > we should be able to accomplish this:
> > > >
> > > > 1. Make the Perl integration tests actually make HTTP calls which can
> > > > then be handled by traffic_ops_golang, rather than calling the Perl
> > > > API router directly.
> > > > 2. Remove test interdependencies by mocking out the test data.
> > > > 3. Rewrite all the Perl tests in the go API test framework and remove
> > > > the Perl tests.
> > > >
> > > > We are also open to other suggestions that allow us to remove dead
> Perl
> > > > code :).
> > > >
> > > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > > >
> > > > - Rawlin
> > > >
> > >
> >
>

Re: Removing dead Perl code

Posted by Dave Neuman <ne...@apache.org>.
+1 on removing dead code.
I think I am mostly in agreement with Rob, I think we should remove the
dead code and delete the Perl tests that break as a result.  In my opinion
the tests aren't very useful as they are pretty fagile (hard-coded array
numbers) and aren't doing that much validation anyway.  If we can come up
with a reasonable way to replace a perl test with a test in golang that
exercises the Perl API then we should attempt to do it.  As we re-write
endpoints in go, we should make sure that we write useful tests using the
testing framework including writing whatever needs to be written in the
golang client to support the test.

Thanks,
Dave


On Wed, Jun 20, 2018 at 10:50 AM Dan Kirkwood <da...@gmail.com> wrote:

> I'm all for #3.   In fact,  the `traffic_ops/testing/api` framework that
> Dewayne has developed encourages testing from the HTTP calls (which you
> suggest in #1).  That covers the endpoint no matter how it's implemented.
>
> Removing dead Perl code can be tricky,  since the interdependencies are not
> always obvious, and can lead to unforeseen missing functionality.  TO uses
> dynamic addition of methods to objects.  The Mojolicious framework, in
> fact, encourages this.   I'm in favor of getting better testing in place
> that covers existing endpoints no matter where they fall.  As the Go code
> progresses,  remove the endpoints from Perl (but not the supporting code)
> and remove tests as they are added on the Go side.   Remove the whole Perl
> code once the Go code is complete.
>
> -dan
>
> On Wed, Jun 20, 2018 at 10:18 AM Jeremy Mitchell <mi...@gmail.com>
> wrote:
>
> > #1 is pretty hard / not feasible. I believe Dewayne already looked into
> > that
> > #2 sounds hard and probably work that would just be wasted imo
> > #3 i like this idea. let's pretend we have 100 API tests in perl. rewrite
> > all 100 to Go, delete the perl tests entirely. this is also a good idea
> > because it provides somewhat of a safety net for the perl to Go api
> rewrite
> > which i don't think exists currently.
> >
> > Jeremy
> >
> > On Wed, Jun 20, 2018 at 9:54 AM, Rawlin Peters <ra...@gmail.com>
> > wrote:
> >
> > > Hey Traffic Controllers,
> > >
> > > In order to accelerate our progress toward using and developing
> > > traffic_ops_golang as a community, I'd like to propose that we remove
> > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > > have been rewritten in Go at this point, and by keeping the obsolete
> > > Perl endpoints in the repo we're not making it clear that new
> > > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > > in order to actually work and survive long-term (as opposed to the
> > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > > and removed in the near future).
> > >
> > > Right now the only thing keeping the dead Perl endpoints from being
> > > deleted is the Perl test framework that depends on them. Unfortunately
> > > the tests are all caught in a spider web of interdependency, so we
> > > can't simply remove tests for APIs that have been rewritten in Go
> > > without breaking other tests. However, we think there are a few ways
> > > we should be able to accomplish this:
> > >
> > > 1. Make the Perl integration tests actually make HTTP calls which can
> > > then be handled by traffic_ops_golang, rather than calling the Perl
> > > API router directly.
> > > 2. Remove test interdependencies by mocking out the test data.
> > > 3. Rewrite all the Perl tests in the go API test framework and remove
> > > the Perl tests.
> > >
> > > We are also open to other suggestions that allow us to remove dead Perl
> > > code :).
> > >
> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > >
> > > - Rawlin
> > >
> >
>

Re: Removing dead Perl code

Posted by Dan Kirkwood <da...@gmail.com>.
I'm all for #3.   In fact,  the `traffic_ops/testing/api` framework that
Dewayne has developed encourages testing from the HTTP calls (which you
suggest in #1).  That covers the endpoint no matter how it's implemented.

Removing dead Perl code can be tricky,  since the interdependencies are not
always obvious, and can lead to unforeseen missing functionality.  TO uses
dynamic addition of methods to objects.  The Mojolicious framework, in
fact, encourages this.   I'm in favor of getting better testing in place
that covers existing endpoints no matter where they fall.  As the Go code
progresses,  remove the endpoints from Perl (but not the supporting code)
and remove tests as they are added on the Go side.   Remove the whole Perl
code once the Go code is complete.

-dan

On Wed, Jun 20, 2018 at 10:18 AM Jeremy Mitchell <mi...@gmail.com>
wrote:

> #1 is pretty hard / not feasible. I believe Dewayne already looked into
> that
> #2 sounds hard and probably work that would just be wasted imo
> #3 i like this idea. let's pretend we have 100 API tests in perl. rewrite
> all 100 to Go, delete the perl tests entirely. this is also a good idea
> because it provides somewhat of a safety net for the perl to Go api rewrite
> which i don't think exists currently.
>
> Jeremy
>
> On Wed, Jun 20, 2018 at 9:54 AM, Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > Hey Traffic Controllers,
> >
> > In order to accelerate our progress toward using and developing
> > traffic_ops_golang as a community, I'd like to propose that we remove
> > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > have been rewritten in Go at this point, and by keeping the obsolete
> > Perl endpoints in the repo we're not making it clear that new
> > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > in order to actually work and survive long-term (as opposed to the
> > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > and removed in the near future).
> >
> > Right now the only thing keeping the dead Perl endpoints from being
> > deleted is the Perl test framework that depends on them. Unfortunately
> > the tests are all caught in a spider web of interdependency, so we
> > can't simply remove tests for APIs that have been rewritten in Go
> > without breaking other tests. However, we think there are a few ways
> > we should be able to accomplish this:
> >
> > 1. Make the Perl integration tests actually make HTTP calls which can
> > then be handled by traffic_ops_golang, rather than calling the Perl
> > API router directly.
> > 2. Remove test interdependencies by mocking out the test data.
> > 3. Rewrite all the Perl tests in the go API test framework and remove
> > the Perl tests.
> >
> > We are also open to other suggestions that allow us to remove dead Perl
> > code :).
> >
> > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> >
> > - Rawlin
> >
>

Re: Removing dead Perl code

Posted by Robert Butts <ro...@apache.org>.
+1 on removing dead code.

IMO the danger and cost of the dead code far outweighs the benefit of the
Perl tests. Especially considering AFAIK most of the tests are just
verifying the response is a 200, not actually checking the body or database
or validating anything.

We just had someone in the community write code, for the second time, in
old dead Perl that's never called. We're wasting people's time, dangers and
bugs aside.

I vote we remove the dead code ASAP, and then worry about adding more,
useful tests.

>#1 is pretty hard / not feasible. I believe Dewayne already looked into
that
>#2 sounds hard and probably work that would just be wasted imo
> #3 i like this idea. let's pretend we have 100 API tests in perl. rewrite
>all 100 to Go, delete the perl tests entirely. this is also a good idea
>because it provides somewhat of a safety net for the perl to Go api rewrite
>which i don't think exists currently.

2 sounds a lot easier than 3 to me. I agree completely, we need to write
"API Tests" for all the Go endpoints. But the "API Test" framework takes a
lot of time, in my experience, to work with. It doesn't support a lot of
things (like Riak). In the past, when I've converted an endpoint,
frequently the framework hasn't supported all the tables it's hit, or
external resources, or some combination, and it's required significant work
to extend. Further, the "API Test" framework uses the Go client, which is
missing a huge number of endpoints. Which means writing tests for endpoints
requires additionally writing client functions. Extending the client is
good, but it's more time, and delays the Go rewrite project.

Yes, 2 would be "wasted" in the long term, but I suspect it's far less work
for each endpoint, to replace calls to removed endpoints/tests with mock
data. I could be wrong, I haven't investigated thoroughly, but I'm hoping
to look into it more shortly.

I'm not convinced 1 is infeasible either. It seems like the kind of thing
that could appear difficult/impossible until you stumble on the one magic
way to make it work. It might be worth another person or two independently
investigating, to see if someone stumbles across the magic way to do it.


But again, my vote would be to delete the dead Perl code now, delete tests
that break, that arguably aren't very useful anyway, and write new "API
Tests" as we go, and devote our development time to getting the rewrite
done. It's a huge task, and every hour we devote to maintaining the old
minimally-useful Perl tests is that much longer before the project is done.

But if there's consensus that we can't do that, I vote we do whatever is
the least work, and I suspect that's #2.


On Wed, Jun 20, 2018 at 10:18 AM, Jeremy Mitchell <mi...@gmail.com>
wrote:

> #1 is pretty hard / not feasible. I believe Dewayne already looked into
> that
> #2 sounds hard and probably work that would just be wasted imo
> #3 i like this idea. let's pretend we have 100 API tests in perl. rewrite
> all 100 to Go, delete the perl tests entirely. this is also a good idea
> because it provides somewhat of a safety net for the perl to Go api rewrite
> which i don't think exists currently.
>
> Jeremy
>
> On Wed, Jun 20, 2018 at 9:54 AM, Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > Hey Traffic Controllers,
> >
> > In order to accelerate our progress toward using and developing
> > traffic_ops_golang as a community, I'd like to propose that we remove
> > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > have been rewritten in Go at this point, and by keeping the obsolete
> > Perl endpoints in the repo we're not making it clear that new
> > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > in order to actually work and survive long-term (as opposed to the
> > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > and removed in the near future).
> >
> > Right now the only thing keeping the dead Perl endpoints from being
> > deleted is the Perl test framework that depends on them. Unfortunately
> > the tests are all caught in a spider web of interdependency, so we
> > can't simply remove tests for APIs that have been rewritten in Go
> > without breaking other tests. However, we think there are a few ways
> > we should be able to accomplish this:
> >
> > 1. Make the Perl integration tests actually make HTTP calls which can
> > then be handled by traffic_ops_golang, rather than calling the Perl
> > API router directly.
> > 2. Remove test interdependencies by mocking out the test data.
> > 3. Rewrite all the Perl tests in the go API test framework and remove
> > the Perl tests.
> >
> > We are also open to other suggestions that allow us to remove dead Perl
> > code :).
> >
> > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> >
> > - Rawlin
> >
>

Re: Removing dead Perl code

Posted by Jeremy Mitchell <mi...@gmail.com>.
#1 is pretty hard / not feasible. I believe Dewayne already looked into that
#2 sounds hard and probably work that would just be wasted imo
#3 i like this idea. let's pretend we have 100 API tests in perl. rewrite
all 100 to Go, delete the perl tests entirely. this is also a good idea
because it provides somewhat of a safety net for the perl to Go api rewrite
which i don't think exists currently.

Jeremy

On Wed, Jun 20, 2018 at 9:54 AM, Rawlin Peters <ra...@gmail.com>
wrote:

> Hey Traffic Controllers,
>
> In order to accelerate our progress toward using and developing
> traffic_ops_golang as a community, I'd like to propose that we remove
> all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> have been rewritten in Go at this point, and by keeping the obsolete
> Perl endpoints in the repo we're not making it clear that new
> enhancements have to be made in traffic_ops_golang and Traffic Portal
> in order to actually work and survive long-term (as opposed to the
> legacy Perl Traffic Ops API and UI which is planned to be deprecated
> and removed in the near future).
>
> Right now the only thing keeping the dead Perl endpoints from being
> deleted is the Perl test framework that depends on them. Unfortunately
> the tests are all caught in a spider web of interdependency, so we
> can't simply remove tests for APIs that have been rewritten in Go
> without breaking other tests. However, we think there are a few ways
> we should be able to accomplish this:
>
> 1. Make the Perl integration tests actually make HTTP calls which can
> then be handled by traffic_ops_golang, rather than calling the Perl
> API router directly.
> 2. Remove test interdependencies by mocking out the test data.
> 3. Rewrite all the Perl tests in the go API test framework and remove
> the Perl tests.
>
> We are also open to other suggestions that allow us to remove dead Perl
> code :).
>
> Please +1 or -1 if you agree or disagree; all feedback is welcome.
>
> - Rawlin
>

Re: Removing dead Perl code

Posted by Dewayne Richardson <de...@apache.org>.
> So, there might be a middle ground. How about this: what if we remove the
> Perl routes from the routes file, and change the functions to immediately
> return an informative error message? That way, if we miss some dynamic
Perl
> still calling the code, we immediately get a useful message, so it's easy
> to find and fix. And we don't call old, wrong code that could do something
> dangerous and bad. And additionally, contributors in the community won't
> see the functions that look real, and accidentally waste their time
>  modifying them.

I agree with this, but the question becomes do the deprecated responses
return an error?  Or just a new section in the response that says something
like:

"info": {
"details": "Please use route /api/..."
"deprecated": true
}

But I do think until the rewrite is done this is our best option.

-Dew

On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:

> >as long as we have 100% feature complete with the Go code (which I think
> we're fairly close?)
> We are not close. We are at 108 of 240 endpoints.
>
> >if we do not treat the tests as first class citizens (meaning we implement
> an Go endpoint and build coverage as we go), then why bother at all?
> It doesn't have to be all or nothing. There's a happy medium between
> requiring tests for every line of code, and having no tests.
>
> >I also thought that we were supporting removing the Perl in the next TC
> 3.0 major release (several emails back).
> That vote was to remove the Perl GUI, not all of Perl.
>
>
> >Removing dead Perl code can be tricky,  since the interdependencies are
> not always obvious, and can lead to unforeseen missing functionality.
>
> That's a good point. But let me ask this: in the event we miss some dynamic
> Perl calling a function that was removed, which is worse: to make that
> function fail, or to make that function call dead code, which has been
> changed and modified to behave differently in Go, and the undead Perl is
> now doing the wrong thing?
>
>
> So, there might be a middle ground. How about this: what if we remove the
> Perl routes from the routes file, and change the functions to immediately
> return an informative error message? That way, if we miss some dynamic Perl
> still calling the code, we immediately get a useful message, so it's easy
> to find and fix. And we don't call old, wrong code that could do something
> dangerous and bad. And additionally, contributors in the community won't
> see the functions that look real, and accidentally waste their time
> modifying them.
>
> Those who voted against removing the dead code, does that sound like a
> compromise you could live with?
>
>
> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <de...@apache.org>
> wrote:
>
> > I'm +100 on removing dead Perl code as long as we have 100% feature
> > complete with the Go code (which I think we're fairly close?).  Rawlin
> you
> > are correct in that there is a spider web of interdependencies in Perl,
> but
> > without feature parity removing any of that dead code is like pulling a
> > single thread from a sweater without unraveling the entire thing.  In
> other
> > words let's put our efforts around Go features and moving forward.
> >
> > As for tests, I realize writing tests "takes time", but we have to build
> a
> > safety net for the Traffic Ops Go API's going forward.  I also realize
> that
> > writing and maintaining the API tests is like maintaining two code bases,
> > but if we do not treat the tests as first class citizens (meaning we
> > implement an Go endpoint and build coverage as we go), then why bother at
> > all?  I'm pretty sure there is support for more testing and I also think
> it
> > has to be a "community" effort.
> >
> > I also thought that we were supporting removing the Perl in the next TC
> 3.0
> > major release (several emails back).
> >
> > -Dew
> >
> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <ra...@gmail.com>
> > wrote:
> >
> > > Hey Traffic Controllers,
> > >
> > > In order to accelerate our progress toward using and developing
> > > traffic_ops_golang as a community, I'd like to propose that we remove
> > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > > have been rewritten in Go at this point, and by keeping the obsolete
> > > Perl endpoints in the repo we're not making it clear that new
> > > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > > in order to actually work and survive long-term (as opposed to the
> > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > > and removed in the near future).
> > >
> > > Right now the only thing keeping the dead Perl endpoints from being
> > > deleted is the Perl test framework that depends on them. Unfortunately
> > > the tests are all caught in a spider web of interdependency, so we
> > > can't simply remove tests for APIs that have been rewritten in Go
> > > without breaking other tests. However, we think there are a few ways
> > > we should be able to accomplish this:
> > >
> > > 1. Make the Perl integration tests actually make HTTP calls which can
> > > then be handled by traffic_ops_golang, rather than calling the Perl
> > > API router directly.
> > > 2. Remove test interdependencies by mocking out the test data.
> > > 3. Rewrite all the Perl tests in the go API test framework and remove
> > > the Perl tests.
> > >
> > > We are also open to other suggestions that allow us to remove dead Perl
> > > code :).
> > >
> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > >
> > > - Rawlin
> > >
> >
>

Re: Removing dead Perl code

Posted by Rawlin Peters <ra...@gmail.com>.
The attachment didn't send; here's a pastebin of the same output:
https://pastebin.com/Lj1cH4p6.

- Rawlin
On Mon, Jul 2, 2018 at 9:06 AM Rawlin Peters <ra...@gmail.com> wrote:
>
> I just discovered that there are a handful of Perl TO UI pages that
> also depend on Perl API endpoints (screenshot attached). So, I think
> we should either:
> 1. Fix the Perl TO UI pages to make requests to traffic_ops_golang
> (just changing the URL from /api/1.1/cachegroups.json to
> http://localhost:<traffic_ops_golang port>/api/1.1/cachegroups.json
> doesn't work; TO golang gives the error "error getting cookie: http:
> named cookie not present")
> 2. Remove these Perl TO UI pages that have a direct equivalent in
> Traffic Portal (which properly makes requests to traffic_ops_golang)
> 3. Accept that obsolete Perl UI pages can be "broken" (since they're
> dependent upon Perl endpoints that have been rewritten in Go) and
> display a warning on the Perl UI page
>
> My vote would be for option 2 because option 1 would be wasted effort
> long-term, and it should be very easy to verify that TP has all the
> same functionality as the TO UI equivalent before removing a page. I
> know we're really close to getting TP parity and have discussed
> removing the entire Perl UI after a major release or two, but I think
> we can reap some immediate benefits by removing obsolete TO UI pages
> that have a direct equivalent in TP now (at least the ones in the
> attached screenshot) without having to delete the entire TO UI. For
> instance, if an endpoint has already been rewritten in
> traffic_ops_golang, you wouldn't have to update both implementations
> of the API just to keep the old Perl UI intact.
>
> Maybe we could even replace the obsolete pages with a configurable
> link to the TP equivalent. We'd get the benefit of having more users
> migrate to TP who could help find gaps in parity. We'd also somewhat
> alleviate the issue of PRs implementing new features in Perl rather
> than Go and TP.
>
> Does anyone see any problems with this option (2)?
>
> - Rawlin
>
> On Tue, Jun 26, 2018 at 9:48 AM Dewayne Richardson <de...@apache.org> wrote:
> >
> > At this point, I think the Perl tests have rotted so much and I don't want
> > to put anymore effort into them.  I say as long as we have Go coverage
> > (because the API tests will still go through to Perl if no Go catches the
> > route), then I'm good.
> >
> > -Dew
> >
> > On Tue, Jun 26, 2018 at 8:14 AM Robert Butts <ro...@apache.org> wrote:
> >
> > > I had planned on deleting tests for deleted functions (which have tests in
> > > Go), and starting with functions whose tests aren't depended on by any
> > > other tests, and working our way up. Unless anyone objects.
> > >
> > >
> > > On Mon, Jun 25, 2018 at 8:17 AM, Rawlin Peters <ra...@gmail.com>
> > > wrote:
> > >
> > > > Alright, so we have a compromise on the dead Perl API code, but what
> > > > about the Perl tests that use the routed functions we're changing to
> > > > just return an error? We can remove the Perl tests that test the dead
> > > > endpoints, but what about any live Perl endpoints that depend on dead
> > > > ones?
> > > > Move the Perl API code into a new test-only function and call that
> > > > from the test?
> > > > Just delete the test because it depends on a dead endpoint?
> > > > Delete and rewrite it in the Go testing framework?
> > > >
> > > > - Rawlin
> > > >
> > > > On Sun, Jun 24, 2018 at 1:45 PM, Dan Kirkwood <da...@gmail.com> wrote:
> > > > > On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:
> > > > >
> > > > >> >as long as we have 100% feature complete with the Go code (which I
> > > > think
> > > > >> we're fairly close?)
> > > > >> We are not close. We are at 108 of 240 endpoints.
> > > > >>
> > > > >> >if we do not treat the tests as first class citizens (meaning we
> > > > implement
> > > > >> an Go endpoint and build coverage as we go), then why bother at all?
> > > > >> It doesn't have to be all or nothing. There's a happy medium between
> > > > >> requiring tests for every line of code, and having no tests.
> > > > >>
> > > > >> >I also thought that we were supporting removing the Perl in the next
> > > TC
> > > > >> 3.0 major release (several emails back).
> > > > >> That vote was to remove the Perl GUI, not all of Perl.
> > > > >>
> > > > >>
> > > > >> >Removing dead Perl code can be tricky,  since the interdependencies
> > > are
> > > > >> not always obvious, and can lead to unforeseen missing functionality.
> > > > >>
> > > > >> That's a good point. But let me ask this: in the event we miss some
> > > > dynamic
> > > > >> Perl calling a function that was removed, which is worse: to make that
> > > > >> function fail, or to make that function call dead code, which has been
> > > > >> changed and modified to behave differently in Go, and the undead Perl
> > > is
> > > > >> now doing the wrong thing?
> > > > >>
> > > > >
> > > > > In the case where a function is removed that overrode some default
> > > > > functionality (which happens a lot in this code),  the default code
> > > would
> > > > > not fail, but produce different results.  Not quite helpful..
> > >  However,
> > > > > the idea you present below mitigates that risk..
> > > > >
> > > > >
> > > > >> So, there might be a middle ground. How about this: what if we remove
> > > > the
> > > > >> Perl routes from the routes file, and change the functions to
> > > > immediately
> > > > >> return an informative error message? That way, if we miss some dynamic
> > > > Perl
> > > > >> still calling the code, we immediately get a useful message, so it's
> > > > easy
> > > > >> to find and fix. And we don't call old, wrong code that could do
> > > > something
> > > > >> dangerous and bad. And additionally, contributors in the community
> > > won't
> > > > >> see the functions that look real, and accidentally waste their time
> > > > >> modifying them.
> > > > >>
> > > > >
> > > > > This,   I can live with,  and I think it represents a good way forward:
> > > > >  keep the signature of all functions -- do not remove any modules and
> > > > > replace the guts of each of those functions with a sensible error --
> > > even
> > > > > with a stack trace so we can tell exactly what code path was used to
> > > get
> > > > > there.   It should be fairly straightforward to create a utility
> > > function
> > > > > that can be used throughout so it happens consistently.
> > > > >
> > > > >
> > > > >
> > > > >> Those who voted against removing the dead code, does that sound like a
> > > > >> compromise you could live with?
> > > > >
> > > > >
> > > > > That's a yes from me.   Thanks for coming up with a solid compromise..
> > > > >
> > > > > -Dan
> > > > >
> > > > >
> > > > >> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <
> > > dewrich@apache.org
> > > > >
> > > > >> wrote:
> > > > >>
> > > > >> > I'm +100 on removing dead Perl code as long as we have 100% feature
> > > > >> > complete with the Go code (which I think we're fairly close?).
> > > Rawlin
> > > > >> you
> > > > >> > are correct in that there is a spider web of interdependencies in
> > > > Perl,
> > > > >> but
> > > > >> > without feature parity removing any of that dead code is like
> > > pulling
> > > > a
> > > > >> > single thread from a sweater without unraveling the entire thing.
> > > In
> > > > >> other
> > > > >> > words let's put our efforts around Go features and moving forward.
> > > > >> >
> > > > >> > As for tests, I realize writing tests "takes time", but we have to
> > > > build
> > > > >> a
> > > > >> > safety net for the Traffic Ops Go API's going forward.  I also
> > > realize
> > > > >> that
> > > > >> > writing and maintaining the API tests is like maintaining two code
> > > > bases,
> > > > >> > but if we do not treat the tests as first class citizens (meaning we
> > > > >> > implement an Go endpoint and build coverage as we go), then why
> > > > bother at
> > > > >> > all?  I'm pretty sure there is support for more testing and I also
> > > > think
> > > > >> it
> > > > >> > has to be a "community" effort.
> > > > >> >
> > > > >> > I also thought that we were supporting removing the Perl in the next
> > > > TC
> > > > >> 3.0
> > > > >> > major release (several emails back).
> > > > >> >
> > > > >> > -Dew
> > > > >> >
> > > > >> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <
> > > > rawlin.peters@gmail.com>
> > > > >> > wrote:
> > > > >> >
> > > > >> > > Hey Traffic Controllers,
> > > > >> > >
> > > > >> > > In order to accelerate our progress toward using and developing
> > > > >> > > traffic_ops_golang as a community, I'd like to propose that we
> > > > remove
> > > > >> > > all of the dead Perl Traffic Ops API code in the repo. Many
> > > > endpoints
> > > > >> > > have been rewritten in Go at this point, and by keeping the
> > > obsolete
> > > > >> > > Perl endpoints in the repo we're not making it clear that new
> > > > >> > > enhancements have to be made in traffic_ops_golang and Traffic
> > > > Portal
> > > > >> > > in order to actually work and survive long-term (as opposed to the
> > > > >> > > legacy Perl Traffic Ops API and UI which is planned to be
> > > deprecated
> > > > >> > > and removed in the near future).
> > > > >> > >
> > > > >> > > Right now the only thing keeping the dead Perl endpoints from
> > > being
> > > > >> > > deleted is the Perl test framework that depends on them.
> > > > Unfortunately
> > > > >> > > the tests are all caught in a spider web of interdependency, so we
> > > > >> > > can't simply remove tests for APIs that have been rewritten in Go
> > > > >> > > without breaking other tests. However, we think there are a few
> > > ways
> > > > >> > > we should be able to accomplish this:
> > > > >> > >
> > > > >> > > 1. Make the Perl integration tests actually make HTTP calls which
> > > > can
> > > > >> > > then be handled by traffic_ops_golang, rather than calling the
> > > Perl
> > > > >> > > API router directly.
> > > > >> > > 2. Remove test interdependencies by mocking out the test data.
> > > > >> > > 3. Rewrite all the Perl tests in the go API test framework and
> > > > remove
> > > > >> > > the Perl tests.
> > > > >> > >
> > > > >> > > We are also open to other suggestions that allow us to remove dead
> > > > Perl
> > > > >> > > code :).
> > > > >> > >
> > > > >> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > > > >> > >
> > > > >> > > - Rawlin
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >

Re: Removing dead Perl code

Posted by Rawlin Peters <ra...@gmail.com>.
I just discovered that there are a handful of Perl TO UI pages that
also depend on Perl API endpoints (screenshot attached). So, I think
we should either:
1. Fix the Perl TO UI pages to make requests to traffic_ops_golang
(just changing the URL from /api/1.1/cachegroups.json to
http://localhost:<traffic_ops_golang port>/api/1.1/cachegroups.json
doesn't work; TO golang gives the error "error getting cookie: http:
named cookie not present")
2. Remove these Perl TO UI pages that have a direct equivalent in
Traffic Portal (which properly makes requests to traffic_ops_golang)
3. Accept that obsolete Perl UI pages can be "broken" (since they're
dependent upon Perl endpoints that have been rewritten in Go) and
display a warning on the Perl UI page

My vote would be for option 2 because option 1 would be wasted effort
long-term, and it should be very easy to verify that TP has all the
same functionality as the TO UI equivalent before removing a page. I
know we're really close to getting TP parity and have discussed
removing the entire Perl UI after a major release or two, but I think
we can reap some immediate benefits by removing obsolete TO UI pages
that have a direct equivalent in TP now (at least the ones in the
attached screenshot) without having to delete the entire TO UI. For
instance, if an endpoint has already been rewritten in
traffic_ops_golang, you wouldn't have to update both implementations
of the API just to keep the old Perl UI intact.

Maybe we could even replace the obsolete pages with a configurable
link to the TP equivalent. We'd get the benefit of having more users
migrate to TP who could help find gaps in parity. We'd also somewhat
alleviate the issue of PRs implementing new features in Perl rather
than Go and TP.

Does anyone see any problems with this option (2)?

- Rawlin

On Tue, Jun 26, 2018 at 9:48 AM Dewayne Richardson <de...@apache.org> wrote:
>
> At this point, I think the Perl tests have rotted so much and I don't want
> to put anymore effort into them.  I say as long as we have Go coverage
> (because the API tests will still go through to Perl if no Go catches the
> route), then I'm good.
>
> -Dew
>
> On Tue, Jun 26, 2018 at 8:14 AM Robert Butts <ro...@apache.org> wrote:
>
> > I had planned on deleting tests for deleted functions (which have tests in
> > Go), and starting with functions whose tests aren't depended on by any
> > other tests, and working our way up. Unless anyone objects.
> >
> >
> > On Mon, Jun 25, 2018 at 8:17 AM, Rawlin Peters <ra...@gmail.com>
> > wrote:
> >
> > > Alright, so we have a compromise on the dead Perl API code, but what
> > > about the Perl tests that use the routed functions we're changing to
> > > just return an error? We can remove the Perl tests that test the dead
> > > endpoints, but what about any live Perl endpoints that depend on dead
> > > ones?
> > > Move the Perl API code into a new test-only function and call that
> > > from the test?
> > > Just delete the test because it depends on a dead endpoint?
> > > Delete and rewrite it in the Go testing framework?
> > >
> > > - Rawlin
> > >
> > > On Sun, Jun 24, 2018 at 1:45 PM, Dan Kirkwood <da...@gmail.com> wrote:
> > > > On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:
> > > >
> > > >> >as long as we have 100% feature complete with the Go code (which I
> > > think
> > > >> we're fairly close?)
> > > >> We are not close. We are at 108 of 240 endpoints.
> > > >>
> > > >> >if we do not treat the tests as first class citizens (meaning we
> > > implement
> > > >> an Go endpoint and build coverage as we go), then why bother at all?
> > > >> It doesn't have to be all or nothing. There's a happy medium between
> > > >> requiring tests for every line of code, and having no tests.
> > > >>
> > > >> >I also thought that we were supporting removing the Perl in the next
> > TC
> > > >> 3.0 major release (several emails back).
> > > >> That vote was to remove the Perl GUI, not all of Perl.
> > > >>
> > > >>
> > > >> >Removing dead Perl code can be tricky,  since the interdependencies
> > are
> > > >> not always obvious, and can lead to unforeseen missing functionality.
> > > >>
> > > >> That's a good point. But let me ask this: in the event we miss some
> > > dynamic
> > > >> Perl calling a function that was removed, which is worse: to make that
> > > >> function fail, or to make that function call dead code, which has been
> > > >> changed and modified to behave differently in Go, and the undead Perl
> > is
> > > >> now doing the wrong thing?
> > > >>
> > > >
> > > > In the case where a function is removed that overrode some default
> > > > functionality (which happens a lot in this code),  the default code
> > would
> > > > not fail, but produce different results.  Not quite helpful..
> >  However,
> > > > the idea you present below mitigates that risk..
> > > >
> > > >
> > > >> So, there might be a middle ground. How about this: what if we remove
> > > the
> > > >> Perl routes from the routes file, and change the functions to
> > > immediately
> > > >> return an informative error message? That way, if we miss some dynamic
> > > Perl
> > > >> still calling the code, we immediately get a useful message, so it's
> > > easy
> > > >> to find and fix. And we don't call old, wrong code that could do
> > > something
> > > >> dangerous and bad. And additionally, contributors in the community
> > won't
> > > >> see the functions that look real, and accidentally waste their time
> > > >> modifying them.
> > > >>
> > > >
> > > > This,   I can live with,  and I think it represents a good way forward:
> > > >  keep the signature of all functions -- do not remove any modules and
> > > > replace the guts of each of those functions with a sensible error --
> > even
> > > > with a stack trace so we can tell exactly what code path was used to
> > get
> > > > there.   It should be fairly straightforward to create a utility
> > function
> > > > that can be used throughout so it happens consistently.
> > > >
> > > >
> > > >
> > > >> Those who voted against removing the dead code, does that sound like a
> > > >> compromise you could live with?
> > > >
> > > >
> > > > That's a yes from me.   Thanks for coming up with a solid compromise..
> > > >
> > > > -Dan
> > > >
> > > >
> > > >> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <
> > dewrich@apache.org
> > > >
> > > >> wrote:
> > > >>
> > > >> > I'm +100 on removing dead Perl code as long as we have 100% feature
> > > >> > complete with the Go code (which I think we're fairly close?).
> > Rawlin
> > > >> you
> > > >> > are correct in that there is a spider web of interdependencies in
> > > Perl,
> > > >> but
> > > >> > without feature parity removing any of that dead code is like
> > pulling
> > > a
> > > >> > single thread from a sweater without unraveling the entire thing.
> > In
> > > >> other
> > > >> > words let's put our efforts around Go features and moving forward.
> > > >> >
> > > >> > As for tests, I realize writing tests "takes time", but we have to
> > > build
> > > >> a
> > > >> > safety net for the Traffic Ops Go API's going forward.  I also
> > realize
> > > >> that
> > > >> > writing and maintaining the API tests is like maintaining two code
> > > bases,
> > > >> > but if we do not treat the tests as first class citizens (meaning we
> > > >> > implement an Go endpoint and build coverage as we go), then why
> > > bother at
> > > >> > all?  I'm pretty sure there is support for more testing and I also
> > > think
> > > >> it
> > > >> > has to be a "community" effort.
> > > >> >
> > > >> > I also thought that we were supporting removing the Perl in the next
> > > TC
> > > >> 3.0
> > > >> > major release (several emails back).
> > > >> >
> > > >> > -Dew
> > > >> >
> > > >> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <
> > > rawlin.peters@gmail.com>
> > > >> > wrote:
> > > >> >
> > > >> > > Hey Traffic Controllers,
> > > >> > >
> > > >> > > In order to accelerate our progress toward using and developing
> > > >> > > traffic_ops_golang as a community, I'd like to propose that we
> > > remove
> > > >> > > all of the dead Perl Traffic Ops API code in the repo. Many
> > > endpoints
> > > >> > > have been rewritten in Go at this point, and by keeping the
> > obsolete
> > > >> > > Perl endpoints in the repo we're not making it clear that new
> > > >> > > enhancements have to be made in traffic_ops_golang and Traffic
> > > Portal
> > > >> > > in order to actually work and survive long-term (as opposed to the
> > > >> > > legacy Perl Traffic Ops API and UI which is planned to be
> > deprecated
> > > >> > > and removed in the near future).
> > > >> > >
> > > >> > > Right now the only thing keeping the dead Perl endpoints from
> > being
> > > >> > > deleted is the Perl test framework that depends on them.
> > > Unfortunately
> > > >> > > the tests are all caught in a spider web of interdependency, so we
> > > >> > > can't simply remove tests for APIs that have been rewritten in Go
> > > >> > > without breaking other tests. However, we think there are a few
> > ways
> > > >> > > we should be able to accomplish this:
> > > >> > >
> > > >> > > 1. Make the Perl integration tests actually make HTTP calls which
> > > can
> > > >> > > then be handled by traffic_ops_golang, rather than calling the
> > Perl
> > > >> > > API router directly.
> > > >> > > 2. Remove test interdependencies by mocking out the test data.
> > > >> > > 3. Rewrite all the Perl tests in the go API test framework and
> > > remove
> > > >> > > the Perl tests.
> > > >> > >
> > > >> > > We are also open to other suggestions that allow us to remove dead
> > > Perl
> > > >> > > code :).
> > > >> > >
> > > >> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > > >> > >
> > > >> > > - Rawlin
> > > >> > >
> > > >> >
> > > >>
> > >
> >

Re: Removing dead Perl code

Posted by Dewayne Richardson <de...@apache.org>.
At this point, I think the Perl tests have rotted so much and I don't want
to put anymore effort into them.  I say as long as we have Go coverage
(because the API tests will still go through to Perl if no Go catches the
route), then I'm good.

-Dew

On Tue, Jun 26, 2018 at 8:14 AM Robert Butts <ro...@apache.org> wrote:

> I had planned on deleting tests for deleted functions (which have tests in
> Go), and starting with functions whose tests aren't depended on by any
> other tests, and working our way up. Unless anyone objects.
>
>
> On Mon, Jun 25, 2018 at 8:17 AM, Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > Alright, so we have a compromise on the dead Perl API code, but what
> > about the Perl tests that use the routed functions we're changing to
> > just return an error? We can remove the Perl tests that test the dead
> > endpoints, but what about any live Perl endpoints that depend on dead
> > ones?
> > Move the Perl API code into a new test-only function and call that
> > from the test?
> > Just delete the test because it depends on a dead endpoint?
> > Delete and rewrite it in the Go testing framework?
> >
> > - Rawlin
> >
> > On Sun, Jun 24, 2018 at 1:45 PM, Dan Kirkwood <da...@gmail.com> wrote:
> > > On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:
> > >
> > >> >as long as we have 100% feature complete with the Go code (which I
> > think
> > >> we're fairly close?)
> > >> We are not close. We are at 108 of 240 endpoints.
> > >>
> > >> >if we do not treat the tests as first class citizens (meaning we
> > implement
> > >> an Go endpoint and build coverage as we go), then why bother at all?
> > >> It doesn't have to be all or nothing. There's a happy medium between
> > >> requiring tests for every line of code, and having no tests.
> > >>
> > >> >I also thought that we were supporting removing the Perl in the next
> TC
> > >> 3.0 major release (several emails back).
> > >> That vote was to remove the Perl GUI, not all of Perl.
> > >>
> > >>
> > >> >Removing dead Perl code can be tricky,  since the interdependencies
> are
> > >> not always obvious, and can lead to unforeseen missing functionality.
> > >>
> > >> That's a good point. But let me ask this: in the event we miss some
> > dynamic
> > >> Perl calling a function that was removed, which is worse: to make that
> > >> function fail, or to make that function call dead code, which has been
> > >> changed and modified to behave differently in Go, and the undead Perl
> is
> > >> now doing the wrong thing?
> > >>
> > >
> > > In the case where a function is removed that overrode some default
> > > functionality (which happens a lot in this code),  the default code
> would
> > > not fail, but produce different results.  Not quite helpful..
>  However,
> > > the idea you present below mitigates that risk..
> > >
> > >
> > >> So, there might be a middle ground. How about this: what if we remove
> > the
> > >> Perl routes from the routes file, and change the functions to
> > immediately
> > >> return an informative error message? That way, if we miss some dynamic
> > Perl
> > >> still calling the code, we immediately get a useful message, so it's
> > easy
> > >> to find and fix. And we don't call old, wrong code that could do
> > something
> > >> dangerous and bad. And additionally, contributors in the community
> won't
> > >> see the functions that look real, and accidentally waste their time
> > >> modifying them.
> > >>
> > >
> > > This,   I can live with,  and I think it represents a good way forward:
> > >  keep the signature of all functions -- do not remove any modules and
> > > replace the guts of each of those functions with a sensible error --
> even
> > > with a stack trace so we can tell exactly what code path was used to
> get
> > > there.   It should be fairly straightforward to create a utility
> function
> > > that can be used throughout so it happens consistently.
> > >
> > >
> > >
> > >> Those who voted against removing the dead code, does that sound like a
> > >> compromise you could live with?
> > >
> > >
> > > That's a yes from me.   Thanks for coming up with a solid compromise..
> > >
> > > -Dan
> > >
> > >
> > >> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <
> dewrich@apache.org
> > >
> > >> wrote:
> > >>
> > >> > I'm +100 on removing dead Perl code as long as we have 100% feature
> > >> > complete with the Go code (which I think we're fairly close?).
> Rawlin
> > >> you
> > >> > are correct in that there is a spider web of interdependencies in
> > Perl,
> > >> but
> > >> > without feature parity removing any of that dead code is like
> pulling
> > a
> > >> > single thread from a sweater without unraveling the entire thing.
> In
> > >> other
> > >> > words let's put our efforts around Go features and moving forward.
> > >> >
> > >> > As for tests, I realize writing tests "takes time", but we have to
> > build
> > >> a
> > >> > safety net for the Traffic Ops Go API's going forward.  I also
> realize
> > >> that
> > >> > writing and maintaining the API tests is like maintaining two code
> > bases,
> > >> > but if we do not treat the tests as first class citizens (meaning we
> > >> > implement an Go endpoint and build coverage as we go), then why
> > bother at
> > >> > all?  I'm pretty sure there is support for more testing and I also
> > think
> > >> it
> > >> > has to be a "community" effort.
> > >> >
> > >> > I also thought that we were supporting removing the Perl in the next
> > TC
> > >> 3.0
> > >> > major release (several emails back).
> > >> >
> > >> > -Dew
> > >> >
> > >> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <
> > rawlin.peters@gmail.com>
> > >> > wrote:
> > >> >
> > >> > > Hey Traffic Controllers,
> > >> > >
> > >> > > In order to accelerate our progress toward using and developing
> > >> > > traffic_ops_golang as a community, I'd like to propose that we
> > remove
> > >> > > all of the dead Perl Traffic Ops API code in the repo. Many
> > endpoints
> > >> > > have been rewritten in Go at this point, and by keeping the
> obsolete
> > >> > > Perl endpoints in the repo we're not making it clear that new
> > >> > > enhancements have to be made in traffic_ops_golang and Traffic
> > Portal
> > >> > > in order to actually work and survive long-term (as opposed to the
> > >> > > legacy Perl Traffic Ops API and UI which is planned to be
> deprecated
> > >> > > and removed in the near future).
> > >> > >
> > >> > > Right now the only thing keeping the dead Perl endpoints from
> being
> > >> > > deleted is the Perl test framework that depends on them.
> > Unfortunately
> > >> > > the tests are all caught in a spider web of interdependency, so we
> > >> > > can't simply remove tests for APIs that have been rewritten in Go
> > >> > > without breaking other tests. However, we think there are a few
> ways
> > >> > > we should be able to accomplish this:
> > >> > >
> > >> > > 1. Make the Perl integration tests actually make HTTP calls which
> > can
> > >> > > then be handled by traffic_ops_golang, rather than calling the
> Perl
> > >> > > API router directly.
> > >> > > 2. Remove test interdependencies by mocking out the test data.
> > >> > > 3. Rewrite all the Perl tests in the go API test framework and
> > remove
> > >> > > the Perl tests.
> > >> > >
> > >> > > We are also open to other suggestions that allow us to remove dead
> > Perl
> > >> > > code :).
> > >> > >
> > >> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > >> > >
> > >> > > - Rawlin
> > >> > >
> > >> >
> > >>
> >
>

Re: Removing dead Perl code

Posted by Robert Butts <ro...@apache.org>.
I had planned on deleting tests for deleted functions (which have tests in
Go), and starting with functions whose tests aren't depended on by any
other tests, and working our way up. Unless anyone objects.


On Mon, Jun 25, 2018 at 8:17 AM, Rawlin Peters <ra...@gmail.com>
wrote:

> Alright, so we have a compromise on the dead Perl API code, but what
> about the Perl tests that use the routed functions we're changing to
> just return an error? We can remove the Perl tests that test the dead
> endpoints, but what about any live Perl endpoints that depend on dead
> ones?
> Move the Perl API code into a new test-only function and call that
> from the test?
> Just delete the test because it depends on a dead endpoint?
> Delete and rewrite it in the Go testing framework?
>
> - Rawlin
>
> On Sun, Jun 24, 2018 at 1:45 PM, Dan Kirkwood <da...@gmail.com> wrote:
> > On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:
> >
> >> >as long as we have 100% feature complete with the Go code (which I
> think
> >> we're fairly close?)
> >> We are not close. We are at 108 of 240 endpoints.
> >>
> >> >if we do not treat the tests as first class citizens (meaning we
> implement
> >> an Go endpoint and build coverage as we go), then why bother at all?
> >> It doesn't have to be all or nothing. There's a happy medium between
> >> requiring tests for every line of code, and having no tests.
> >>
> >> >I also thought that we were supporting removing the Perl in the next TC
> >> 3.0 major release (several emails back).
> >> That vote was to remove the Perl GUI, not all of Perl.
> >>
> >>
> >> >Removing dead Perl code can be tricky,  since the interdependencies are
> >> not always obvious, and can lead to unforeseen missing functionality.
> >>
> >> That's a good point. But let me ask this: in the event we miss some
> dynamic
> >> Perl calling a function that was removed, which is worse: to make that
> >> function fail, or to make that function call dead code, which has been
> >> changed and modified to behave differently in Go, and the undead Perl is
> >> now doing the wrong thing?
> >>
> >
> > In the case where a function is removed that overrode some default
> > functionality (which happens a lot in this code),  the default code would
> > not fail, but produce different results.  Not quite helpful..   However,
> > the idea you present below mitigates that risk..
> >
> >
> >> So, there might be a middle ground. How about this: what if we remove
> the
> >> Perl routes from the routes file, and change the functions to
> immediately
> >> return an informative error message? That way, if we miss some dynamic
> Perl
> >> still calling the code, we immediately get a useful message, so it's
> easy
> >> to find and fix. And we don't call old, wrong code that could do
> something
> >> dangerous and bad. And additionally, contributors in the community won't
> >> see the functions that look real, and accidentally waste their time
> >> modifying them.
> >>
> >
> > This,   I can live with,  and I think it represents a good way forward:
> >  keep the signature of all functions -- do not remove any modules and
> > replace the guts of each of those functions with a sensible error -- even
> > with a stack trace so we can tell exactly what code path was used to get
> > there.   It should be fairly straightforward to create a utility function
> > that can be used throughout so it happens consistently.
> >
> >
> >
> >> Those who voted against removing the dead code, does that sound like a
> >> compromise you could live with?
> >
> >
> > That's a yes from me.   Thanks for coming up with a solid compromise..
> >
> > -Dan
> >
> >
> >> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <dewrich@apache.org
> >
> >> wrote:
> >>
> >> > I'm +100 on removing dead Perl code as long as we have 100% feature
> >> > complete with the Go code (which I think we're fairly close?).  Rawlin
> >> you
> >> > are correct in that there is a spider web of interdependencies in
> Perl,
> >> but
> >> > without feature parity removing any of that dead code is like pulling
> a
> >> > single thread from a sweater without unraveling the entire thing.  In
> >> other
> >> > words let's put our efforts around Go features and moving forward.
> >> >
> >> > As for tests, I realize writing tests "takes time", but we have to
> build
> >> a
> >> > safety net for the Traffic Ops Go API's going forward.  I also realize
> >> that
> >> > writing and maintaining the API tests is like maintaining two code
> bases,
> >> > but if we do not treat the tests as first class citizens (meaning we
> >> > implement an Go endpoint and build coverage as we go), then why
> bother at
> >> > all?  I'm pretty sure there is support for more testing and I also
> think
> >> it
> >> > has to be a "community" effort.
> >> >
> >> > I also thought that we were supporting removing the Perl in the next
> TC
> >> 3.0
> >> > major release (several emails back).
> >> >
> >> > -Dew
> >> >
> >> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <
> rawlin.peters@gmail.com>
> >> > wrote:
> >> >
> >> > > Hey Traffic Controllers,
> >> > >
> >> > > In order to accelerate our progress toward using and developing
> >> > > traffic_ops_golang as a community, I'd like to propose that we
> remove
> >> > > all of the dead Perl Traffic Ops API code in the repo. Many
> endpoints
> >> > > have been rewritten in Go at this point, and by keeping the obsolete
> >> > > Perl endpoints in the repo we're not making it clear that new
> >> > > enhancements have to be made in traffic_ops_golang and Traffic
> Portal
> >> > > in order to actually work and survive long-term (as opposed to the
> >> > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> >> > > and removed in the near future).
> >> > >
> >> > > Right now the only thing keeping the dead Perl endpoints from being
> >> > > deleted is the Perl test framework that depends on them.
> Unfortunately
> >> > > the tests are all caught in a spider web of interdependency, so we
> >> > > can't simply remove tests for APIs that have been rewritten in Go
> >> > > without breaking other tests. However, we think there are a few ways
> >> > > we should be able to accomplish this:
> >> > >
> >> > > 1. Make the Perl integration tests actually make HTTP calls which
> can
> >> > > then be handled by traffic_ops_golang, rather than calling the Perl
> >> > > API router directly.
> >> > > 2. Remove test interdependencies by mocking out the test data.
> >> > > 3. Rewrite all the Perl tests in the go API test framework and
> remove
> >> > > the Perl tests.
> >> > >
> >> > > We are also open to other suggestions that allow us to remove dead
> Perl
> >> > > code :).
> >> > >
> >> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> >> > >
> >> > > - Rawlin
> >> > >
> >> >
> >>
>

Re: Removing dead Perl code

Posted by Rawlin Peters <ra...@gmail.com>.
Alright, so we have a compromise on the dead Perl API code, but what
about the Perl tests that use the routed functions we're changing to
just return an error? We can remove the Perl tests that test the dead
endpoints, but what about any live Perl endpoints that depend on dead
ones?
Move the Perl API code into a new test-only function and call that
from the test?
Just delete the test because it depends on a dead endpoint?
Delete and rewrite it in the Go testing framework?

- Rawlin

On Sun, Jun 24, 2018 at 1:45 PM, Dan Kirkwood <da...@gmail.com> wrote:
> On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:
>
>> >as long as we have 100% feature complete with the Go code (which I think
>> we're fairly close?)
>> We are not close. We are at 108 of 240 endpoints.
>>
>> >if we do not treat the tests as first class citizens (meaning we implement
>> an Go endpoint and build coverage as we go), then why bother at all?
>> It doesn't have to be all or nothing. There's a happy medium between
>> requiring tests for every line of code, and having no tests.
>>
>> >I also thought that we were supporting removing the Perl in the next TC
>> 3.0 major release (several emails back).
>> That vote was to remove the Perl GUI, not all of Perl.
>>
>>
>> >Removing dead Perl code can be tricky,  since the interdependencies are
>> not always obvious, and can lead to unforeseen missing functionality.
>>
>> That's a good point. But let me ask this: in the event we miss some dynamic
>> Perl calling a function that was removed, which is worse: to make that
>> function fail, or to make that function call dead code, which has been
>> changed and modified to behave differently in Go, and the undead Perl is
>> now doing the wrong thing?
>>
>
> In the case where a function is removed that overrode some default
> functionality (which happens a lot in this code),  the default code would
> not fail, but produce different results.  Not quite helpful..   However,
> the idea you present below mitigates that risk..
>
>
>> So, there might be a middle ground. How about this: what if we remove the
>> Perl routes from the routes file, and change the functions to immediately
>> return an informative error message? That way, if we miss some dynamic Perl
>> still calling the code, we immediately get a useful message, so it's easy
>> to find and fix. And we don't call old, wrong code that could do something
>> dangerous and bad. And additionally, contributors in the community won't
>> see the functions that look real, and accidentally waste their time
>> modifying them.
>>
>
> This,   I can live with,  and I think it represents a good way forward:
>  keep the signature of all functions -- do not remove any modules and
> replace the guts of each of those functions with a sensible error -- even
> with a stack trace so we can tell exactly what code path was used to get
> there.   It should be fairly straightforward to create a utility function
> that can be used throughout so it happens consistently.
>
>
>
>> Those who voted against removing the dead code, does that sound like a
>> compromise you could live with?
>
>
> That's a yes from me.   Thanks for coming up with a solid compromise..
>
> -Dan
>
>
>> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <de...@apache.org>
>> wrote:
>>
>> > I'm +100 on removing dead Perl code as long as we have 100% feature
>> > complete with the Go code (which I think we're fairly close?).  Rawlin
>> you
>> > are correct in that there is a spider web of interdependencies in Perl,
>> but
>> > without feature parity removing any of that dead code is like pulling a
>> > single thread from a sweater without unraveling the entire thing.  In
>> other
>> > words let's put our efforts around Go features and moving forward.
>> >
>> > As for tests, I realize writing tests "takes time", but we have to build
>> a
>> > safety net for the Traffic Ops Go API's going forward.  I also realize
>> that
>> > writing and maintaining the API tests is like maintaining two code bases,
>> > but if we do not treat the tests as first class citizens (meaning we
>> > implement an Go endpoint and build coverage as we go), then why bother at
>> > all?  I'm pretty sure there is support for more testing and I also think
>> it
>> > has to be a "community" effort.
>> >
>> > I also thought that we were supporting removing the Perl in the next TC
>> 3.0
>> > major release (several emails back).
>> >
>> > -Dew
>> >
>> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <ra...@gmail.com>
>> > wrote:
>> >
>> > > Hey Traffic Controllers,
>> > >
>> > > In order to accelerate our progress toward using and developing
>> > > traffic_ops_golang as a community, I'd like to propose that we remove
>> > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
>> > > have been rewritten in Go at this point, and by keeping the obsolete
>> > > Perl endpoints in the repo we're not making it clear that new
>> > > enhancements have to be made in traffic_ops_golang and Traffic Portal
>> > > in order to actually work and survive long-term (as opposed to the
>> > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
>> > > and removed in the near future).
>> > >
>> > > Right now the only thing keeping the dead Perl endpoints from being
>> > > deleted is the Perl test framework that depends on them. Unfortunately
>> > > the tests are all caught in a spider web of interdependency, so we
>> > > can't simply remove tests for APIs that have been rewritten in Go
>> > > without breaking other tests. However, we think there are a few ways
>> > > we should be able to accomplish this:
>> > >
>> > > 1. Make the Perl integration tests actually make HTTP calls which can
>> > > then be handled by traffic_ops_golang, rather than calling the Perl
>> > > API router directly.
>> > > 2. Remove test interdependencies by mocking out the test data.
>> > > 3. Rewrite all the Perl tests in the go API test framework and remove
>> > > the Perl tests.
>> > >
>> > > We are also open to other suggestions that allow us to remove dead Perl
>> > > code :).
>> > >
>> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
>> > >
>> > > - Rawlin
>> > >
>> >
>>

Re: Removing dead Perl code

Posted by Dan Kirkwood <da...@gmail.com>.
On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <ro...@apache.org> wrote:

> >as long as we have 100% feature complete with the Go code (which I think
> we're fairly close?)
> We are not close. We are at 108 of 240 endpoints.
>
> >if we do not treat the tests as first class citizens (meaning we implement
> an Go endpoint and build coverage as we go), then why bother at all?
> It doesn't have to be all or nothing. There's a happy medium between
> requiring tests for every line of code, and having no tests.
>
> >I also thought that we were supporting removing the Perl in the next TC
> 3.0 major release (several emails back).
> That vote was to remove the Perl GUI, not all of Perl.
>
>
> >Removing dead Perl code can be tricky,  since the interdependencies are
> not always obvious, and can lead to unforeseen missing functionality.
>
> That's a good point. But let me ask this: in the event we miss some dynamic
> Perl calling a function that was removed, which is worse: to make that
> function fail, or to make that function call dead code, which has been
> changed and modified to behave differently in Go, and the undead Perl is
> now doing the wrong thing?
>

In the case where a function is removed that overrode some default
functionality (which happens a lot in this code),  the default code would
not fail, but produce different results.  Not quite helpful..   However,
the idea you present below mitigates that risk..


> So, there might be a middle ground. How about this: what if we remove the
> Perl routes from the routes file, and change the functions to immediately
> return an informative error message? That way, if we miss some dynamic Perl
> still calling the code, we immediately get a useful message, so it's easy
> to find and fix. And we don't call old, wrong code that could do something
> dangerous and bad. And additionally, contributors in the community won't
> see the functions that look real, and accidentally waste their time
> modifying them.
>

This,   I can live with,  and I think it represents a good way forward:
 keep the signature of all functions -- do not remove any modules and
replace the guts of each of those functions with a sensible error -- even
with a stack trace so we can tell exactly what code path was used to get
there.   It should be fairly straightforward to create a utility function
that can be used throughout so it happens consistently.



> Those who voted against removing the dead code, does that sound like a
> compromise you could live with?


That's a yes from me.   Thanks for coming up with a solid compromise..

-Dan


> On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <de...@apache.org>
> wrote:
>
> > I'm +100 on removing dead Perl code as long as we have 100% feature
> > complete with the Go code (which I think we're fairly close?).  Rawlin
> you
> > are correct in that there is a spider web of interdependencies in Perl,
> but
> > without feature parity removing any of that dead code is like pulling a
> > single thread from a sweater without unraveling the entire thing.  In
> other
> > words let's put our efforts around Go features and moving forward.
> >
> > As for tests, I realize writing tests "takes time", but we have to build
> a
> > safety net for the Traffic Ops Go API's going forward.  I also realize
> that
> > writing and maintaining the API tests is like maintaining two code bases,
> > but if we do not treat the tests as first class citizens (meaning we
> > implement an Go endpoint and build coverage as we go), then why bother at
> > all?  I'm pretty sure there is support for more testing and I also think
> it
> > has to be a "community" effort.
> >
> > I also thought that we were supporting removing the Perl in the next TC
> 3.0
> > major release (several emails back).
> >
> > -Dew
> >
> > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <ra...@gmail.com>
> > wrote:
> >
> > > Hey Traffic Controllers,
> > >
> > > In order to accelerate our progress toward using and developing
> > > traffic_ops_golang as a community, I'd like to propose that we remove
> > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > > have been rewritten in Go at this point, and by keeping the obsolete
> > > Perl endpoints in the repo we're not making it clear that new
> > > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > > in order to actually work and survive long-term (as opposed to the
> > > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > > and removed in the near future).
> > >
> > > Right now the only thing keeping the dead Perl endpoints from being
> > > deleted is the Perl test framework that depends on them. Unfortunately
> > > the tests are all caught in a spider web of interdependency, so we
> > > can't simply remove tests for APIs that have been rewritten in Go
> > > without breaking other tests. However, we think there are a few ways
> > > we should be able to accomplish this:
> > >
> > > 1. Make the Perl integration tests actually make HTTP calls which can
> > > then be handled by traffic_ops_golang, rather than calling the Perl
> > > API router directly.
> > > 2. Remove test interdependencies by mocking out the test data.
> > > 3. Rewrite all the Perl tests in the go API test framework and remove
> > > the Perl tests.
> > >
> > > We are also open to other suggestions that allow us to remove dead Perl
> > > code :).
> > >
> > > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> > >
> > > - Rawlin
> > >
> >
>

Re: Removing dead Perl code

Posted by Robert Butts <ro...@apache.org>.
>as long as we have 100% feature complete with the Go code (which I think
we're fairly close?)
We are not close. We are at 108 of 240 endpoints.

>if we do not treat the tests as first class citizens (meaning we implement
an Go endpoint and build coverage as we go), then why bother at all?
It doesn't have to be all or nothing. There's a happy medium between
requiring tests for every line of code, and having no tests.

>I also thought that we were supporting removing the Perl in the next TC
3.0 major release (several emails back).
That vote was to remove the Perl GUI, not all of Perl.


>Removing dead Perl code can be tricky,  since the interdependencies are
not always obvious, and can lead to unforeseen missing functionality.

That's a good point. But let me ask this: in the event we miss some dynamic
Perl calling a function that was removed, which is worse: to make that
function fail, or to make that function call dead code, which has been
changed and modified to behave differently in Go, and the undead Perl is
now doing the wrong thing?


So, there might be a middle ground. How about this: what if we remove the
Perl routes from the routes file, and change the functions to immediately
return an informative error message? That way, if we miss some dynamic Perl
still calling the code, we immediately get a useful message, so it's easy
to find and fix. And we don't call old, wrong code that could do something
dangerous and bad. And additionally, contributors in the community won't
see the functions that look real, and accidentally waste their time
modifying them.

Those who voted against removing the dead code, does that sound like a
compromise you could live with?


On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <de...@apache.org>
wrote:

> I'm +100 on removing dead Perl code as long as we have 100% feature
> complete with the Go code (which I think we're fairly close?).  Rawlin you
> are correct in that there is a spider web of interdependencies in Perl, but
> without feature parity removing any of that dead code is like pulling a
> single thread from a sweater without unraveling the entire thing.  In other
> words let's put our efforts around Go features and moving forward.
>
> As for tests, I realize writing tests "takes time", but we have to build a
> safety net for the Traffic Ops Go API's going forward.  I also realize that
> writing and maintaining the API tests is like maintaining two code bases,
> but if we do not treat the tests as first class citizens (meaning we
> implement an Go endpoint and build coverage as we go), then why bother at
> all?  I'm pretty sure there is support for more testing and I also think it
> has to be a "community" effort.
>
> I also thought that we were supporting removing the Perl in the next TC 3.0
> major release (several emails back).
>
> -Dew
>
> On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > Hey Traffic Controllers,
> >
> > In order to accelerate our progress toward using and developing
> > traffic_ops_golang as a community, I'd like to propose that we remove
> > all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> > have been rewritten in Go at this point, and by keeping the obsolete
> > Perl endpoints in the repo we're not making it clear that new
> > enhancements have to be made in traffic_ops_golang and Traffic Portal
> > in order to actually work and survive long-term (as opposed to the
> > legacy Perl Traffic Ops API and UI which is planned to be deprecated
> > and removed in the near future).
> >
> > Right now the only thing keeping the dead Perl endpoints from being
> > deleted is the Perl test framework that depends on them. Unfortunately
> > the tests are all caught in a spider web of interdependency, so we
> > can't simply remove tests for APIs that have been rewritten in Go
> > without breaking other tests. However, we think there are a few ways
> > we should be able to accomplish this:
> >
> > 1. Make the Perl integration tests actually make HTTP calls which can
> > then be handled by traffic_ops_golang, rather than calling the Perl
> > API router directly.
> > 2. Remove test interdependencies by mocking out the test data.
> > 3. Rewrite all the Perl tests in the go API test framework and remove
> > the Perl tests.
> >
> > We are also open to other suggestions that allow us to remove dead Perl
> > code :).
> >
> > Please +1 or -1 if you agree or disagree; all feedback is welcome.
> >
> > - Rawlin
> >
>

Re: Removing dead Perl code

Posted by Dewayne Richardson <de...@apache.org>.
I'm +100 on removing dead Perl code as long as we have 100% feature
complete with the Go code (which I think we're fairly close?).  Rawlin you
are correct in that there is a spider web of interdependencies in Perl, but
without feature parity removing any of that dead code is like pulling a
single thread from a sweater without unraveling the entire thing.  In other
words let's put our efforts around Go features and moving forward.

As for tests, I realize writing tests "takes time", but we have to build a
safety net for the Traffic Ops Go API's going forward.  I also realize that
writing and maintaining the API tests is like maintaining two code bases,
but if we do not treat the tests as first class citizens (meaning we
implement an Go endpoint and build coverage as we go), then why bother at
all?  I'm pretty sure there is support for more testing and I also think it
has to be a "community" effort.

I also thought that we were supporting removing the Perl in the next TC 3.0
major release (several emails back).

-Dew

On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <ra...@gmail.com>
wrote:

> Hey Traffic Controllers,
>
> In order to accelerate our progress toward using and developing
> traffic_ops_golang as a community, I'd like to propose that we remove
> all of the dead Perl Traffic Ops API code in the repo. Many endpoints
> have been rewritten in Go at this point, and by keeping the obsolete
> Perl endpoints in the repo we're not making it clear that new
> enhancements have to be made in traffic_ops_golang and Traffic Portal
> in order to actually work and survive long-term (as opposed to the
> legacy Perl Traffic Ops API and UI which is planned to be deprecated
> and removed in the near future).
>
> Right now the only thing keeping the dead Perl endpoints from being
> deleted is the Perl test framework that depends on them. Unfortunately
> the tests are all caught in a spider web of interdependency, so we
> can't simply remove tests for APIs that have been rewritten in Go
> without breaking other tests. However, we think there are a few ways
> we should be able to accomplish this:
>
> 1. Make the Perl integration tests actually make HTTP calls which can
> then be handled by traffic_ops_golang, rather than calling the Perl
> API router directly.
> 2. Remove test interdependencies by mocking out the test data.
> 3. Rewrite all the Perl tests in the go API test framework and remove
> the Perl tests.
>
> We are also open to other suggestions that allow us to remove dead Perl
> code :).
>
> Please +1 or -1 if you agree or disagree; all feedback is welcome.
>
> - Rawlin
>