Hey Rawlin- Both good points worth some serious thought. > On Nov 12, 2018, at 1:29 PM, 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). EF> Yeah, the YAML parsing was the only thing I saw that was not a perfect fit for bash. Given how concise that db.yaml file that is, I wouldn’t think twice about getting the open line via: $ DB=“development” grep -A2 “$DB_NAME:” dbconf.yml | grep open | awk -F ":" '{print $2}'
No special tool needed. Is pyyaml part of the batteries-included packages? If not, we’ll need a way to distribute pyyaml.whl as part of the traffic ops RPM. (For those of us without permitted Internet access in our deployments, installing via pip means standing up our own private PyPi repo) at each of our customers- something I would really like to avoid. > > 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. EF>All the web_deps stuff should go away with the rest of perl, right? We really should be targeting RPMs that contain all dependencies (or can be resolved via yum/rpm), rather than asking our users to install stuff from the Internet at install time. Its fragile (packages disappear) and a bunch of TC users do not run in environments with access to the net. I’m not opposed to Python, its probably my favorite language, certainly the one I'm most comfortable in. I just don't see a compelling need for it with these changes. At some point soon we will need to rewrite perl scripts into something else (postinstall, ORT, etc…). We should closely consider our use of language for those as well- Go, Python, bash, etc… —Eric > > - 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 >>>>> >>>>> >>>>> >>>> >>
