> There is a lot of code in that package... it might be helpful to precisely pint out what exactly would be deprecated
Fair enough. I just figured anyone who'd want to vote would already know what it is, but the exact exported symbols from that package that we're talking about here are: • GenericCreator • GenericReader • GenericUpdater • GenericDeleter • GenericOptionsDeleter • GenericCreate • GenericCreateNameBasedID • TryIfModifiedSinceQuery • GenericRead • GenericUpdate • GenericOptionsDelete • GenericDelete • KeyFieldInfo • GetIntKey • GetStringKey • ReadHandler • DeprecatedReadHandler • UpdateHandler • DeleteHandler • DeprecatedDeleteHandler • CreateHandler • CRUDer • AlertsResponse • Identifier • Creator • MultipleCreator • Reader • Updater • Deleter • OptionsDeleter • Tenantable • APIInfoer > Does it make sense to ever rewrite simple > APIs like those as-is? I strongly agree routes shouldn't be rewritten > until they're sufficiently complex to warrant converting it from > "Generic CRUDer", but with simple routes like those that will likely > never see any new additions, wouldn't that mean "Generic CRUDer" will > live on as perpetually deprecated? yeah, probably. I don't really have a problem with that. If that code never has to change, then the fact that it uses this old, deprecated framework shouldn't have the chance to bother anyone. On Wed, Oct 14, 2020 at 3:54 PM Rawlin Peters <[email protected]> wrote: > There is a lot of code in that package [1]. It's hard to agree to the > deprecation of "the Generic CRUDer framework" when it's not clear what > that actually includes (especially for people who don't have detailed > knowledge of that code). Just so everyone is on the same page, it > might be helpful to precisely point out what exactly would be > deprecated per this proposal. Then, we can more easily discuss the > pros/cons of using "the Generic CRUDer framework" vs. the other > method(s). Otherwise, I'm really not sure what we're discussing. > > I will say, the "framework"-y stuff in TO has its pros and cons, but > it does tend to take care of a decent amount of boilerplate code. If > we're talking about the same "framework" (we might not be), it does > pretty well at handling simple routes like /divisions [2]. For about > ~100 LOC you get four fully-functioning routes, fully equipped with > decoding, validation, error handling, change logging, etc. Basically > all you have to provide is your SQL queries, and you get most of the > API functionality for free. Does it make sense to ever rewrite simple > APIs like those as-is? I strongly agree routes shouldn't be rewritten > until they're sufficiently complex to warrant converting it from > "Generic CRUDer", but with simple routes like those that will likely > never see any new additions, wouldn't that mean "Generic CRUDer" will > live on as perpetually deprecated? > > - Rawlin > > [1] > https://github.com/apache/trafficcontrol/tree/master/traffic_ops/traffic_ops_golang/api > [2] > https://github.com/apache/trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/division/divisions.go > > On Wed, Oct 14, 2020 at 1:59 PM ocket 8888 <[email protected]> wrote: > > > > At yesterday's working group, we agreed that none of us like the "Generic > > CRUDer" framework provided by the > > github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api > > package. So if nobody has any objections, we'd like to deprecate it. > > > > That would entail GoDoc deprecation notices, a note in the > > documentation/README, and advising authors of new API endpoints against > > using it. Existing endpoints would be rewritten to not use the CRUDer as > > new features are requested of them that rise to a level of complexity > that > > warrants "CRUDer stripping" at the author's and reviewer's discretions. >
