Make HTTPTransport compatible with multiple API versions
#228
Conversation
There was a problem hiding this comment.
GTM, left a comment about how to test if we're at the lowest protocol version available, other than that it's OK I think (this is clearly not tested in detail on CI as we do not yet support old or beta agent versions, @palazzem has queued work about this AFAIR). Side note: looks like on CI test failed...
lib/ddtrace/transport.rb
Outdated
| @@ -119,6 +137,8 @@ def server_error?(code) | |||
| # endpoint. In both cases, we're going to downgrade the transporter encoder so that | |||
| # it will target a stable API. | |||
| def downgrade?(code) | |||
| return if @api.fetch(:compatibility_mode) | |||
There was a problem hiding this comment.
Should we test compatibility_mode here or the existence of a fallback? Both are equivalent with our data, so this is just a nitpick but I was worried because I could not see the check on the existence of an available fallback.
There was a problem hiding this comment.
humn, makes sense. I think I'll get rid of the compatibility_mode altogether and just check for the existence of a fallback!
There was a problem hiding this comment.
Yes the compatibility_mode was added to the Python client because at that time we had a different data model + JSON/Msgpack encoders. Now, nobody uses such old trace-agent. We can just use the fallback.
p-lambert
force-pushed
the
pedro/multiple-api-versions
branch
from
37021f6 to
6c51291
Compare
p-lambert
force-pushed
the
pedro/multiple-api-versions
branch
from
6c51291 to
f95170a
Compare
No description provided.