I think Go really is the best path forward for this. A simple Bash script would be nice, but the yaml parsing issue really takes that out of the running IMO. If we added a dbconf.env file with the same data as dbconf.yml except in a format that can be sourced from a Bash script, it would be easy enough to just generate the dbconf.yml file on the fly from the sourced variables in order to use it for the `goose` commands. But then that adds another manual step to the upgrade path where an admin has to either use a tool we provide to convert the dbconf.yml to the Bash source-able file.
But, I think the Bash path is really just a pseudo-low-dependency solution that still probably isn't ideal because it relies on external commands that need to be installed onto the system from somewhere else (probably requiring internet access at install time). I think the postgres commands come from the postgresql96 package. What if we upgrade postgres to the latest version and those external commands are renamed or go away entirely? Ideally I'd think we'd be using the lib/pq and database/sql Go packages to run all the sql files. Then it would be pretty self-contained and probably wouldn't need any changes if we upgraded postgres to a newer version (maybe just need to update the vendored packages and retest). Or if we decide that FooBarDB is actually better than postgres, swapping it out would probably just require changing a package import in the Go tool rather than replacing all the external commands. So to summarize some of what we've discussed so far: - Bash would be nice because it doesn't add a lot of extra dependencies, except it would still rely on installing external commands which might require internet access at install time - Python could also work but requires a bunch of extra build dependencies/steps to essentially "compile" it into a self-contained executable - Go requires a bit more boilerplate for something that would typically just be a Bash script, but Go is already a dependency of Traffic Ops, is used for at least one other TO tool (convert_profile), and would just require vendoring the go-yaml package. It would also give us the opportunity to use native Go DB packages. At this point I think we are really just bikeshedding for something that takes less than a day's work to write, test, and compare functionality with the existing script. I've already done this twice now for both Python and Go, so given what we've discussed so far I'm inclined to just move forward with the Go version unless someone can come up with a compelling reason not to. - Rawlin On Wed, Nov 14, 2018 at 3:31 PM Gray, Jonathan <[email protected]> wrote: > > That project is just a thin wrapper atop github.com/spf13/viper which has a > lot more nuance and power to trip up on if you're not careful. > > Jonathan G > > > On 11/14/18, 2:36 PM, "Dan Kirkwood" <[email protected]> wrote: > > if yaml parsing is the only barrier for bash, try this: > https://github.com/030/go-yq > > > > On Wed, Nov 14, 2018 at 2:18 PM Fieck, Brennan <[email protected]> > wrote: > > > Specifically 2.7 and 3.6? We'd want it to run on 3.4, ideally, but 3.5 > can > > work. Python 2 is going away, and there's no RHEL repo for anything over > > 3.5 afaik. > > ________________________________________ > > From: Chris Lemmons <[email protected]> > > Sent: Wednesday, November 14, 2018 1:41 PM > > To: [email protected] > > Subject: Re: [EXTERNAL] The future of the db/admin.pl script > > > > Yeah, I looked at yq. For one... it's written in Python. :) But it > > does appear to be compatible with 2.7 and 3.6, so it may be easier to > > run. > > On Wed, Nov 14, 2018 at 1:35 PM Dewayne Richardson <[email protected]> > > wrote: > > > > > > No strong opinion here, the admin.pl was built for a different time in > > the > > > history of TC. The reason we used Perl was for config parsing back in > > the > > > day, but there is a yaml version of jq, called yq. I honestly agree > with > > > bash if we can get away with it. > > > > > > FYI: > > > https://github.com/kislyuk/yq > > > > > > -Dew > > > > > > > > > On Mon, Nov 12, 2018 at 11:29 AM Rawlin Peters > <[email protected]> > > > wrote: > > > > > > > Eric, > > > > > > > > I share your sentiment about being reluctant to introduce another > > > > language as a dependency for Traffic Ops, but I wasn't able to find > a > > > > really good, easily-available utility for parsing yaml (a la `jq` > for > > > > json parsing) in a Bash script. Since the goose config is in yaml, > > > > `db/admin.pl` uses a yaml package to parse the goose config into > > > > variables which are then passed to the external `psql` et al. > > > > commands. It is possible to parse yaml using sed, but the example I > > > > found for doing that seemed really sketchy and fragile. So I figured > > > > using a solid YAML-parsing library like PyYAML in Python would be a > > > > safer bet while still allowing the use of a fully-featured > programming > > > > language rather than "Bash + <insert yaml-parsing CLI tool here>". > It > > > > would also allow us to potentially use a DB library to interface > with > > > > the DB directly in Python rather than requiring `psql` et al. and > just > > > > shelling out to those external commands (although I plan to continue > > > > doing it that way for now). > > > > > > > > As a side note, it also paves the way for moving other scripts to > > > > Python like WebDep.pm, which uses a Perl package that is virtually > > > > impossible to install/get running on Mac because of Perl's broken > SSL > > > > on Mac, which would make it much easier to start as a new developer > on > > > > the project. I remember when I started working on Traffic Control, I > > > > had to copy someone else's Perl `traffic_ops/app/local` directory > who > > > > had been on the project a long time and had actually gotten it to > > > > build on Mac before it became unusable. Eliminating issues like that > > > > by using a more popular and supportable language is a win in my > book, > > > > but right now I'm just focusing on `db/admin.pl` to allow for better > > > > testability of the DB migration operations. > > > > > > > > - Rawlin > > > > > > > > On Mon, Nov 12, 2018 at 6:46 AM Eric Friedrich -X (efriedri - TRITON > > > > UK BIDCO LIMITED c/o Alter Domus (UK) Limited -OBO at Cisco) > > > > <[email protected]> wrote: > > > > > > > > > > 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 <[email protected]> > > 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 <[email protected]> > > 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 < > > > > [email protected]> > > > > > >> 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" <[email protected]> > > > > 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 > > > > > >>> > > > > > >>> > > > > > >>> > > > > > >> > > > > > > > > > > > > >
