#Compat for internal go code

1 messages · Page 1 of 1 (latest)

bold falcon
#

So the issue to me seems to be that because we changed that function signature, anyone who calls into the internal/telemetry package and calls .Close is going to hit this? The Bass SDK was affected because it did this, but also we'd have this issue if any other SDK or module called this too.

#

ATM, we're really only doing module compat for the generated code, but the internal/ packages like querybuilder and telemetry should also have compat I guess

#

which is maybe tricky. the way we did compat for those is through templating/etc, but those packages above are just copied directly

#

one option as a short-term solution could be to just not break those modules - e.g. we could have added CloseContext or similar, and then deprecated Close - like good go libraries, but this feels fiddly to do

#

another option would be to fully move telemetry and querybuilder out to a separate repo, version them entirely separately, which i know has been a potential option before?

#

or we introduce magical templating into the generation of those internal packages 😢

bold falcon
heavy adder
# bold falcon *one* option as a short-term solution could be to just *not* break those modules...

I didn't do this originally because passing the ctx is the only correct way to call it now that we install the tracer/logger providers into the ctx for the per-client telemetry DBs in the engine. But you're right, I think we could keep the original Close() without breaking the other use cases, and mark it deprecated, if we just bring back the package-level vars. That begs the question of when we remove the deprecated bit though, bringing us back to square 1 as long as we're in the (noble) mindset of "don't break modules ever."

heavy adder
heavy adder
# bold falcon another option would be to fully move `telemetry` and `querybuilder` out to a se...

Worth a think - it would align with each packages' existing interest of maintaining a tiny set of dependencies. As long as we're able to precisely pin the version expected by the engine's codegen, this seems fine. Only hurdle, I think, is making the separate repos, since I know we've wanted to avoid sprawl in the past. (I would also want one for testctx eventually btw so Go folks have a nice OTel option for testing)