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: [email protected] With regards, Apache Git Services
