On Nov 12, 2018, at 4:03 PM, Rawlin Peters <[email protected]<mailto:[email protected]>> wrote:
replies inline On Mon, Nov 12, 2018 at 11:51 AM Eric Friedrich -X (efriedri - TRITON UK BIDCO LIMITED c/o Alter Domus (UK) Limited -OBO at Cisco) <[email protected]<mailto:[email protected]>> wrote: Hey Rawlin- Both good points worth some serious thought. On Nov 12, 2018, at 1:29 PM, Rawlin Peters <[email protected]<mailto:[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. I wish we could depend on the dbconf.yml file being a standard format like that in order to just be able to grep it with context, but there are no restrictions on empty lines or arbitrary comments in the yaml syntax that would easily break the grep command. We shouldn't have to enforce arbitrary formatting of a yaml config file just to remove the need for a yaml parser. EF> Yeah good point. I rarely touch the file so was not too worried about the syntax. But yeah, if people regularly change that file we should be more permissive with our parsing 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. I don't believe it is part of the standard library, but I do believe we can figure out a way to keep it all self-contained to the traffic ops RPM without requiring internet access at install time. I think https://github.com/pyinstaller/pyinstaller is more than capable of doing that on Linux/Mac/Windows. I just tried it out on my Python version of db/admin.pl on my mac, and it created a self-contained binary of 4.7MB size that seems to work just fine. It does still depend on the external commands like `psql` et al. from the postgres package, but that's no different from the current Perl version. Do you think that would be sufficient? I think pyinstaller also solves the python2 vs python3 availability problem, since the interpreter is packaged into the resulting binary itself. EF> I think that would be OK. The licensing on that one is GPLv2 which is category X to include in source (we can distribute the boot loader though). Do you know of any alternatives with more compatible licensing? 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… I think Go could be a reasonable language for stuff like db/admin.pl, but it seemed more natural to keep scripts as scripts for small stuff like that. - Rawlin
