-
Notifications
You must be signed in to change notification settings - Fork 173
feat(go/adbc/driver/snowflake): Add a static version number #2889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| package adbc | ||
|
|
||
| const DriverVersion = "1.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zeroshade - I am assuming this is the most controversial part of this PR. We track the PREVIOUS_VERSION_NATIVE and increment that when the releases happen, but we don't have anything to indicate the current version. Here I am following the pattern established by the dependency DLLs, but if you have thoughts on better ways to do this, I am all ears.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least from my point of view, this is OK, but I'd rather have it be per-driver rather than in the nominally core library. (We'd also need to update the version bump script to account for this.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to whatever. I based this off of https://github.com/apache/arrow-adbc/releases where it appears all of the Go drivers have the same version (just like C#, where all of the C# drivers end up with the same build version).
|
What's the workflow you're using to build the binary in the pre -production stream? Go should be making the version available via |
We pull the repo and build a NuGet package which internally runs When I ran it in testing, the value of the build info came back as something like |
|
Okay, so we probably aren't using the BuildInfo properly which is why it's getting the "unknown" value. That's easy enough to fix. As for the |
Did you get a chance to trial this @zeroshade ? |
|
Sorry for the delay here, i played with this a bit. It looks like the internal build info doesn't seem to pick up the tags for the non-root module tag. If I add a local tag of In both cases, it should be easy enough to update the local variable to use debug.BuildInfo to get the version, so the question is going to be a matter of updating the build scripts to get it to pick up the version. (Looks like it'll only embed the subdirectory version when using Updating the build script to set the tag before building is still probably better than needing an internal var to update the version etc. Do you want me to update this PR? Or poke it yourself? |
|
I think this is more your area of expertise than mine if you have some time :) |
expanding the solution from #2889 We can make sure that we inject the proper version string into the internal/driverbase handling. This should populate the GetInfo correctly with the driver version via vcs information. --------- Co-authored-by: David Li <[email protected]>
|
I think we can close this now? |
|
I'm gonna close it, if the updates is insufficient, we can file a new issue/PR |
We have pre-production workstreams with the Go drivers in them, but when we need to get the driver version, it always comes back as
(unknown or development build)which makes debugging issues that much more difficult. Dependencies used by the ADBC stack (like gosnowflake and arrow) have a static version that can be used to update the metadata. This PR adds support for a static version number that can be accessed via the metadata call.