Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Update Apache Arrow to v16 #216

Merged
merged 2 commits into from
Apr 29, 2024
Merged

feat: Update Apache Arrow to v16 #216

merged 2 commits into from
Apr 29, 2024

Conversation

candiduslynx
Copy link
Contributor

Instead of #209

@candiduslynx
feat: Update Apache Arrow to
c3edb3b 
Signed-off-by: candiduslynx <candiduslynx@gmail.com>
@candiduslynx candiduslynx mentioned this pull request Apr 12, 2024
Closed
@candiduslynx
Copy link
Contributor Author

@kravets-levko would you mind also reviewing this PR & releasing afterwards?
I've been using a fork at https://github.com/cloudquery/databricks-sql-go (cqmain branch) with #216 & #217 merged into to achieve everything required for the code I'm developing.

The major bump to v15 just updates the library code, the underlying arrow data (wire protocol) stays the same.

@kravets-levko
Copy link
Contributor

@candiduslynx I totally understand your need in this upgrade, however I'm a bit new to this project in particular and Go language in general. So I need @andrefurlan-db or @rcypher-databricks approval, and then I'll merge and ship this change. Sorry, and thank you for understanding

@yunbodeng-db
Copy link
Contributor

@andrefurlan-db can you help out on this one?

@candiduslynx candiduslynx changed the title feat: Update Apache Arrow to v15 feat: Update Apache Arrow to v16 Apr 22, 2024
@candiduslynx
Copy link
Contributor Author

@yunbodeng-db @andrefurlan-db @kravets-levko I've updated the PR as v16 was released.
I'd like to see this merged sooner than later, though.

@candiduslynx
Copy link
Contributor Author

@kravets-levko thanks for the review!
Is there a way to expedite the merge & release of this code along with #207?

@kravets-levko
Copy link
Contributor

kravets-levko commented Apr 25, 2024 •
edited
Loading

@candiduslynx #207 should be already available in v1.5.4. For this PR - I think I'll add it to the next release, hopefully will be available on Monday-Tuesday

@kravets-levko kravets-levko merged commit 700d02f into databricks:main Apr 29, 2024
2 of 3 checks passed
@candiduslynx candiduslynx deleted the feat/arrow/v15 branch April 29, 2024 16:54
@candiduslynx
Copy link
Contributor Author

@kravets-levko it's great to see this merged, thanks!
I'm eagerly waiting for the new release 🙂

@kravets-levko
Copy link
Contributor

@candiduslynx v1.5.5 is out - please give it a try and let me know if everything is okay. Thank you for patience

kravets-levko added a commit that referenced this pull request Jun 4, 2024
@kravets-levko
Revert "feat: Update Apache Arrow to v16 (#216)"
8eb3b20 
This reverts commit 700d02f.
@kravets-levko kravets-levko mentioned this pull request Jun 4, 2024
Merged
@kravets-levko
Copy link
Contributor

kravets-levko commented Jun 4, 2024 •
edited
Loading

@candiduslynx sorry for a bad news, but turns out that Arrow upgrade caused issues for library users. Library offers a feature that allows to access raw Arrow batches, and with this upgrade behavior of that feature changed. After discussion with other team members, we decided to revert this upgrade for now. v1.5.5 will remain unchanged, though, so you can keep using it

kravets-levko added a commit that referenced this pull request Jun 4, 2024
@kravets-levko
Revert update Apache Arrow to v16 (#228)
869c98e 
Reverts #216

Unfortunately, updating Apache Arrow turned out to be a breaking change
which created issues to some library users. We made a decision to revert
this upgrade
@candiduslynx
Copy link
Contributor Author

@kravets-levko thanks for notifying!
I think you need to add some tests to outline the expected behaviour then.
However, I wonder what are the details of the changed behaviour that isn't deemed correct by clients? Is it just a requirement to update the library?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kravets-levko kravets-levko kravets-levko approved these changes

@yunbodeng-db yunbodeng-db Awaiting requested review from yunbodeng-db yunbodeng-db is a code owner

@andrefurlan-db andrefurlan-db Awaiting requested review from andrefurlan-db andrefurlan-db is a code owner

@rcypher-databricks rcypher-databricks Awaiting requested review from rcypher-databricks rcypher-databricks is a code owner

@jadewang-db jadewang-db Awaiting requested review from jadewang-db jadewang-db is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@candiduslynx @kravets-levko @yunbodeng-db