Well said Rawlin. This thread is now 24 replies long, so I think we have enough consensus to just build it in golang and move on.
On Thu, Nov 15, 2018 at 8:58 AM Rawlin Peters <[email protected]> wrote: > 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 > > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >> > > > > > > > > > > > > > > > > > > >
