I’m only slightly familiar with all the different options for db/admin.pl.
I’m a big fan of Python, but reluctant to introduce another language into TC without a strong reason. Once the reverse_schema option is removed, what would be the main purposes of the script? Looks like this is something that could be easily converted to bash without need for another dependency. —Eric > On Nov 10, 2018, at 1:44 PM, Dan Kirkwood <dang...@gmail.com> wrote: > > +1 on rewriting admin.pl -- Python seems a reasonable choice, esp since we > seem to be gaining a lot of Python expertise recently. > > -1 on 2.x compatibility -- writing something new with compatibility for 2 > major versions makes no sense to me. It limits the features and libraries > that can be used and potentially doubles the amount of testing required. > > > > On Sat, Nov 10, 2018 at 8:47 AM Dave Neuman <neu...@apache.org> wrote: > >> +1, seems reasonable. I don’t really have an opinion on python 2.x >> compatibility, but whatever makes the most sense for the amount of work is >> what I would prefer. >> >> On Fri, Nov 9, 2018 at 18:06 Gray, Jonathan <jonathan_g...@comcast.com> >> wrote: >> >>> +1 There is already precedence in the repo for python for other purposes. >>> The only caveat I would include is to be sure you include backward >>> compatibility for python 2.x for the next year or so until it goes EOL. >>> >>> Jonathan G >>> >>> On 11/9/18, 5:23 PM, "Rawlin Peters" <rawlin.pet...@gmail.com> wrote: >>> >>> Hey TC devs, >>> >>> As we eliminate the need for Traffic Ops Perl, it will no longer >>> really make sense for the db/admin.pl script to be Perl as it is >>> today. This is because it depends on the Perl modules that are >>> installed via Carton for running Traffic Ops Perl. However, it's >>> mostly just a Perl wrapper around shell commands except for a YAML >>> Perl module and the `reverse_schema` command which uses the DBIx Perl >>> module to generate the ORM schema for Traffic Ops Perl (i.e. you've >>> added a new DB table/column and need to get the ORM files updated to >>> use it). >>> >>> Without TO-Perl, there's no need for the `reverse_schema` command in >>> db/admin.pl and its dependency on the Perl DBIx module. At that >> point >>> db/admin.pl is just a Perl script that parses some YAML and shells >> out >>> commands. >>> >>> Part of the problem with TO-Perl is that there are a bunch of random >>> non-API Perl scripts that basically assume all the modules in the >>> cpanfile are installed in the environment, even though a lot of those >>> dependencies are probably only required for the Perl TO API or UI. So >>> unless we go through all those Perl dependencies to eliminate >>> everything we don't really need once the Perl TO API and UI are >>> completely removed, we'll continue to have a handful of Perl scripts >>> like db/admin.pl that still require downloading and installing the >>> full set of TO Perl dependencies. On fresh installs, running Carton >> to >>> install these dependencies can take nearly half an hour. >>> >>> I'm working on adding tests for the DB upgrade/downgrade process >> which >>> I'd like to run automatically on PR submissions, but it really only >>> needs the db/admin.pl script out of TO which assumes the entire set >> of >>> Perl TO dependencies even though it mostly just shells out to `goose >>> up` and `goose down`. I'd like the test to emulate an actual TO >>> environment as closely as possible to match what would actually >> happen >>> in a production DB upgrade/downgrade. Right now I'm reusing the >>> Traffic-Ops-Perl Docker image from cdn-in-a-box, but ideally I'd like >>> to port db/admin.pl into Python to give it its own set of >> dependencies >>> (just a YAML package) and not require the full set of TO Perl >>> dependencies. Then I can spin up a much lighter-weight container with >>> just the TO Python packages rather than setting up a separate >> cpanfile >>> with just the Perl packages needed for db/admin.pl. >>> >>> +1/-1 for adding Python as a dependency of Traffic Ops for porting >>> scripts like db/admin.pl? >>> >>> -Rawlin >>> >>> >>> >>
