rawlinp commented on issue #4534: Fix ORT atstccfg to allow using new features 
in the latest Traffic Ops
URL: https://github.com/apache/trafficcontrol/pull/4534#issuecomment-604218435
 
 
   Maybe I overreacted when I saw this in the PR description:
   > it serves as an example of the idiom for future dev.
   
   To me that came across as "all of our Go components should follow this 
example in the future". Those kinds of wide-spread decisions that affect all 
developers in the project need to be communicated so that everyone can get on 
board. However, based on Neuman's comment, it sounds like this is _not_ 
supposed to be a decision like that since it's supposed to be a temporary 
solution just to get ORT to bridge the gap with the 2.0 API. Is that correct 
@rob05c?
   
   Thinking about this more, this seems like an issue that would be permanent 
-- rather than just bridging the gap from 1.4 to 2.0, we will likely need a 
solution to bridge the gap from 2.0 to 2.1, 2.1 to 2.2, and so on, indefinitely.
   
   For example, let's say ORT is using an unvendored TO Go client that's 
currently at 2.0. Traffic Ops is currently running 2.0 in Prod. A dev adds a 
new field for deliveryservices and bumps the version on master to 2.1. The TO 
Go client on master is then updated to request 2.1. A new release is cut from 
master, so now the new build of ORT requests 2.1 as well. However, Prod TO is 
still only running 2.0, and it will 501 any request for 2.1 -- therefore, ORT 
will not function if it's upgraded before TO is.
   
   Using both a vendored and un-vendored version of the Go client _kind of_ 
solves this problem, but the problem with that is that ORT is _not_ vendoring 
the corresponding Go structs. So ORT is making 2.0 requests but unmarshalling 
them into 2.1 structs. Unless ORT also "sanitizes" the 2.1 fields to their 
default values before using them, this approach will lead to some odd bugs 
where ORT assumes the wrong default value. If this is going to be the solution 
going forward, indefinitely, then I think we might have to put some more 
thought into this approach.
   
   If the correct 2.1 defaults are set on the resulting 2.1 struct when a 
request is made to 2.0 but unmarshalled into a 2.1 struct, I think that would 
be fine, but sanitizing defaults seems like something that the client itself 
should handle. In fact, maybe the TO Go client should optionally downgrade the 
request one minor version if it gets back a 501. For instance `GET 
/api/2.1/foos` returned a 501, so optionally retry one time with `GET 
/api/2.0/foos`. If the retry was successful, set the proper 2.1 defaults and 
return the result. However, it wouldn't try to downgrade across major versions 
from e.g. 2.0 to 1.4, since we can't always assume that would be safe.
   
   Would that work going forward from minor version to minor version, instead 
of vendoring the TO Go client indefinitely? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to