@jonathon - i think goose up/down runs migrations and migrations should be limited to structural database changes. If data changes (inserts, updates, deletes, etc) are introduced into migrations then the ability to migrate down can get real tricky. i think that's why patches.sql was introduced for data changes.
Jeremy On Mon, Nov 12, 2018 at 9:27 AM Gray, Jonathan <[email protected]> wrote: > Correct me if I'm wrong, but one of the important functions of the > admin.pl script is the db record seeds. It seems like there's kindof a > separation when comes to upgrading the database that structural changes run > through goose while record initialization runs through admin.pl. I'm not > aware of a reason why goose couldn't insert rows into the database, but I > havn't dug too deep on that. > > Jonathan G > > On 11/12/18, 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 > >>> > >>> > >>> > >> > > > >
