one thought that just came to mind -- both forms are JSON, so TP could easily load in and detect which form it is and produce a "deprecated" warning if the old.
-dan On Tue, Sep 18, 2018 at 8:48 AM Rawlin Peters <[email protected]> wrote: > Mike, > > The old Traffic Ops UI is deprecated and will be removed in 4.0, but > until that point the profile Import/Export in the TO UI should still > continue to work. I think the issue at hand is the Traffic Portal > profile import/export functionality. That will be changed to use the > standard /api/*/profiles JSON format, which will be the standard > import/export format from there on, rather than the legacy format. We > could also add a 2nd button (or format selector) in Traffic Portal for > importing legacy-formatted profiles because that API already exists in > Perl. Maybe we could even detect which format is being imported and > not require the selector in the UI. > > - Rawlin > On Tue, Sep 18, 2018 at 7:50 AM Mike Sandman (misandma) > <[email protected]> wrote: > > > > Sorry, point of clarification - after the proposed change, will the > Import/Export buttons on the Traffic Ops UI still continue to work (but > using a different format)? Or will the Import/Export functionality be gone > from the Traffic Ops UI? > > > > Thanks, > > Mike Sandman > > > > On 9/18/18, 9:33 AM, "Dave Neuman" <[email protected]> wrote: > > > > I am +1 on removing the old Export/Import in favor of using the > API. I > > don't think we need to add a new endpoint to support importing the > old > > version of the export just because someone somewhere might have an > old > > profile export that is not in Traffic Ops anymore and they need to > import > > it. I don't think we should put development effort into supporting > > something that we don't actually know needs to be supported and just > > because we can do something doesn't mean that we should. > > > > On Mon, Sep 17, 2018 at 4:29 PM Robert Butts <[email protected]> wrote: > > > > > >So actually we don't really need to add a new API endpoint at all > > > > > > Sure. I don't object to renaming, if people want to. But the > existing one > > > does work, yes. > > > > > > >Those endpoints should be sufficient enough to support the legacy > format > > > import/export > > > > > > There's no reason to support legacy export, just import. I'm 100% > +1 on > > > deprecating and removing the old export. > > > > > > >Going forward the import/export in TP should be done with the > > > /api/*/profiles format (assuming we add parameters support to that > API), > > > with a separate button for "legacy" import which uses the > deprecated > > > /api/*/profiles/import format > > > > > > +1 > > > > > > >But IMO there's not really a good reason to export profiles and > save them > > > outside of TO for long periods of time > > > > > > I personally don't have a use case either; but I'm sure someone > does, or > > > will. > > > > > > > > > On Mon, Sep 17, 2018 at 4:11 PM Rawlin Peters < > [email protected]> > > > wrote: > > > > > > > So actually we don't really need to add a new API endpoint at > all if > > > > my understanding is correct. We currently have > > > > /api/*/profiles/:id/export and /api/*/profiles/import. Those > endpoints > > > > should be sufficient enough to support the legacy format > import/export > > > > without the need for new endpoints, right? The real issue is > exposing > > > > them via TP. Going forward the import/export in TP should be > done with > > > > the /api/*/profiles format (assuming we add parameters support > to that > > > > API), with a separate button for "legacy" import which uses the > > > > deprecated /api/*/profiles/import format. But IMO there's not > really a > > > > good reason to export profiles and save them outside of TO for > long > > > > periods of time, so I think the whole import/export thing is > really > > > > meant for default profiles or for quickly exporting profiles > from one > > > > TO instance and importing into another. As long as the default > > > > profiles are updated to the new format, I think that would solve > most > > > > of the problem for our users. > > > > > > > > - Rawlin > > > > > > > > On Mon, Sep 17, 2018 at 3:05 PM Robert Butts <[email protected]> > wrote: > > > > > > > > > > >It just seems wrong to me to add a new API endpoint to > support a > > > > > deprecated format. > > > > > > > > > > So people who've exported their data are just out of luck? We > just > > > "don't > > > > > support" migrating your data forward? It seems egregiously > wrong to me > > > > > _not_ to support importing old data, for any app or service, > ever. If > > > we > > > > > gave people a way to create exported data, it's our job to > continue to > > > > > provide them a way to import it. Without them having to pay for > > > > development > > > > > time for an engineer to do a data conversion. > > > > > > > > > > >IMO if we do add a new endpoint just to support the > deprecated legacy > > > > > format, then it should also be marked "deprecated" for removal > in the > > > > next > > > > > release. > > > > > > > > > > The entire point of supporting importing legacy data is _not_ > to > > > > deprecate > > > > > it. This isn't about API versioning, and deprecating old > undesirable > > > > > protocols. That's absolutely the right way to progress the API > itself. > > > > This > > > > > is about providing a way to move data forward. Users have > exported > > > files, > > > > > from an export we gave them. It's the job of any well-behaved > product > > > to > > > > > provide a mechanism to import old formats, when the format > changes. > > > > > > > > > > How would you feel if you were using a product, say Google > Docs, or > > > > > Microsoft Excel, and they suddenly said, "Sorry, your old > documents > > > don't > > > > > work in the latest version." "Oh, but we provide a > command-line script > > > to > > > > > migrate them, you just have to open MS-DOS (or maybe > Powershell?), and > > > > run > > > > > this script, with these arguments. On all your documents." > Sure, as an > > > > > engineer you could do that, however inconvenient; how would > you feel if > > > > you > > > > > read that and were an average user, who'd never seen a command > line > > > > before, > > > > > and only ever used the GUI? > > > > > > > > > > > > > > > On Mon, Sep 17, 2018 at 2:44 PM Rawlin Peters < > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > It just seems wrong to me to add a new API endpoint to > support a > > > > > > deprecated format. There's much more involved in maintaining > an API > > > > > > endpoint as opposed to a one-off script to convert formats. > IMO if we > > > > > > do add a new endpoint just to support the deprecated legacy > format, > > > > > > then it should also be marked "deprecated" for removal in > the next > > > > > > release. > > > > > > > > > > > > - Rawlin > > > > > > > > > > > > On Mon, Sep 17, 2018 at 1:51 PM Robert Butts <[email protected]> > wrote: > > > > > > > > > > > > > > >Do we really want to add a new API endpoint just to > support the > > > > legacy > > > > > > > format > > > > > > > > > > > > > > An endpoint and UI page are easier to use for Self-Service > CDN > > > > consumers. > > > > > > > Some CDN tenants might not know how to run a script or use > a > > > command > > > > > > line. > > > > > > > I'm not sure I understand the objection. Why is it such a > big deal > > > > to add > > > > > > > an endpoint? And/or to support importing old formats? IMO > importing > > > > old > > > > > > > data formats should be a first-class citizen, in any API > or UI. > > > Data > > > > > > jails > > > > > > > are bad. > > > > > > > > > > > > > > >traffic_ops/app/bin/config2json > > > > > > > > > > > > > > The difference is that the TO config is used by system > > > > administrators. As > > > > > > > Self-Service moves forward, profiles and other API data > will be > > > used > > > > by > > > > > > CDN > > > > > > > tenants, who may have very little technical knowledge. We > need to > > > > make > > > > > > sure > > > > > > > anything a Self-Service tenant may need to do has a simple > > > graphical > > > > way > > > > > > to > > > > > > > do it, and also an API so people deploying TC can write > their own > > > > UIs if > > > > > > > necessary. > > > > > > > > > > > > > > > > > > > > > On Mon, Sep 17, 2018 at 1:23 PM Rawlin Peters < > > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > Do we really want to add a new API endpoint just to > support the > > > > legacy > > > > > > > > format? Couldn't we just provide a script that converts > between > > > the > > > > > > > > two formats, similar to what we did for cdn.conf with > > > > > > > > traffic_ops/app/bin/config2json? > > > > > > > > > > > > > > > > - Rawlin > > > > > > > > On Mon, Sep 17, 2018 at 11:04 AM Robert Butts < > [email protected]> > > > > wrote: > > > > > > > > > > > > > > > > > > Ah, excellent. It wasn't clear from the docs that `POST > > > > > > > > /api/1.2/profiles` > > > > > > > > > and `GET /api/1.2/profiles/:id` accept parameters. We > should > > > fix > > > > > > that. > > > > > > > > > > > > > > > > > > That said, I would be in favor of adding a new > endpoint, to > > > > import > > > > > > in the > > > > > > > > > old format, `/api/profile/import-legacy` or something. > Just so > > > > people > > > > > > > > with > > > > > > > > > old exports can import them back in. Data jails are > bad (I know > > > > it's > > > > > > > > > human-readable and not that difficult to convert, but > it'd > > > still > > > > be > > > > > > > > > frustrating). > > > > > > > > > > > > > > > > > > I made Github issues for both: > > > > > > > > > https://github.com/apache/trafficcontrol/issues/2827 > > > > > > > > > https://github.com/apache/trafficcontrol/issues/2826 > > > > > > > > > > > > > > > > > > On Mon, Sep 17, 2018 at 10:08 AM Dan Kirkwood < > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > yes -- that's really what I meant. Import/Export > should not > > > > be a > > > > > > > > separate > > > > > > > > > > operation as documented here: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_ops/default_profiles.html?highlight=import > > > > > > > > > > , > > > > > > > > > > but should be part of the "normal" API. The default > profile > > > > files > > > > > > we > > > > > > > > keep > > > > > > > > > > in > http://trafficcontrol.apache.org/downloads/profiles/ are > > > > in the > > > > > > > > format > > > > > > > > > > I'm describing. They should be in the more > standard form > > > and > > > > > > should > > > > > > > > > > associate all parameters in the file with the > profile. > > > > > > > > > > > > > > > > > > > > The Go `POST api/1.3/profiles` endpoint already > loads the > > > > > > parameters > > > > > > > > into > > > > > > > > > > memory, but ignores the parameters list. > > > > > > > > > > > > > > > > > > > > -dan > > > > > > > > > > > > > > > > > > > > On Mon, Sep 17, 2018 at 9:58 AM Jeremy Mitchell < > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Maybe this is what Dan said but imo there should > be no > > > > > > import/export > > > > > > > > > > > endpoints but these should do the job: > > > > > > > > > > > > > > > > > > > > > > GET /api/*/profiles/:id <-- this is essentially an > export. > > > it > > > > > > gives > > > > > > > > you > > > > > > > > > > the > > > > > > > > > > > json of a profile (along with all the parameters). > save the > > > > json > > > > > > to a > > > > > > > > > > file > > > > > > > > > > > and you've essentially exported the profile. > > > > > > > > > > > POST /api/*/profiles <-- this is how you create > profiles. > > > > if you > > > > > > > > supply > > > > > > > > > > > parameters as well you've essentially done an > import. > > > > > > > > > > > > > > > > > > > > > > jeremy > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Sep 17, 2018 at 9:16 AM Robert Butts < > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Can you clarify, are you referring to > `/profile/import` > > > and > > > > > > > > > > > > `/profile/:id/export`? Or > > > > `/api/$version/profiles/:id/export` > > > > > > and > > > > > > > > > > > > `/api/$version/profiles/import`? > > > > > > > > > > > > > > > > > > > > > > > > We should definitely deprecate and remove the > non-API > > > > > > endpoints. > > > > > > > > But > > > > > > > > > > IMO > > > > > > > > > > > we > > > > > > > > > > > > need to keep a way to create and get an entire > profile, > > > > with > > > > > > > > > > parameters, > > > > > > > > > > > in > > > > > > > > > > > > a single request. Users shouldn't have to make a > dozen > > > > > > requests to > > > > > > > > > > import > > > > > > > > > > > > and export a profile. > > > > > > > > > > > > > > > > > > > > > > > > Also +1 on changing them to real booleans (not > sure if > > > the > > > > api > > > > > > > > > > > > export/import is, they appear to be > undocumented). Though > > > > we > > > > > > can't > > > > > > > > > > break > > > > > > > > > > > > the current API; we could make the POST accept > either, > > > but > > > > the > > > > > > GET > > > > > > > > > > would > > > > > > > > > > > > have to be a new endpoint I think. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Sep 15, 2018 at 3:15 PM Dan Kirkwood < > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > I’d like to propose deprecating the > import/export > > > format > > > > of > > > > > > > > profiles. > > > > > > > > > > > The > > > > > > > > > > > > > current format (Perl-based) is inconsistent > with the > > > > standard > > > > > > > > POST > > > > > > > > > > > > > api/x.x/profiles format. Import/Export > can/should be > > > done > > > > > > using > > > > > > > > the > > > > > > > > > > > same > > > > > > > > > > > > > API. Traffic Portal Import should use the > standard > > > API > > > > > > > > endpoint. > > > > > > > > > > > > > > > > > > > > > > > > > > Import/export (shown below) has this form which > > > includes > > > > the > > > > > > > > > > "profile" > > > > > > > > > > > > key > > > > > > > > > > > > > at the top level. The "secure" option in the > > > > "parameters" > > > > > > secion > > > > > > > > > > uses > > > > > > > > > > > a > > > > > > > > > > > > > 0/1 rather than a boolean true/false. These 2 > things > > > > make it > > > > > > > > > > > > inconsistent. > > > > > > > > > > > > > > > > > > > > > > > > > > Opinions are welcome... > > > > > > > > > > > > > > > > > > > > > > > > > > Dan > > > > > > > > > > > > > > > > > > > > > > > > > > { > > > > > > > > > > > > > "profile": { > > > > > > > > > > > > > "name": "myname", > > > > > > > > > > > > > ... > > > > > > > > > > > > > }, > > > > > > > > > > > > > "parameters": [ > > > > > > > > > > > > > { > > > > > > > > > > > > > "name": "foo", > > > > > > > > > > > > > "secure": 0, > > > > > > > > > > > > > ... > > > > > > > > > > > > > ] > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
