-
Notifications
You must be signed in to change notification settings - Fork 786
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
Donate object_store
code from object_store_rs to arrow-rs
#2081
Conversation
I wonder if we could preserve the history somehow, I'm mainly interested in not losing releases, but this would also help with in-flight PRs. Perhaps you could add object_store_rs as a remote, add a commit moving it to a subdirectory, and then merge it into arrow-rs? |
Update here: my git-fu is not strong enough to try and keep the history from object_store_rs -- @tustvold plans to give it a try. In the interim I will work on porting the tests to use github actions |
Codecov Report
@@ Coverage Diff @@
## master #2081 +/- ##
==========================================
- Coverage 83.71% 82.93% -0.79%
==========================================
Files 225 237 +12
Lines 59632 61322 +1690
==========================================
+ Hits 49923 50855 +932
- Misses 9709 10467 +758
Help us with your feedback. Take ten seconds to tell us how you rate us. |
So unfortunately object_store_rs contains merge commits, so the only way to do this would break the linear history of arrow-rs, either by keeping the merge commits from object_store_rs/main or by adding a single merge commit. I therefore think its not worth it. |
Update on test migration:
|
@tustvold sorry to bother you -- but I was wondering if you had any ideas of ways to port the above test to run on github actions -- specifically it appears that setting the file mode to |
Probably the Github Actions runner is running as root, and therefore the permission failure doesn't occur. I'll rework the test Edit: I can't think of an obvious way around this, I'd just remove the test - it isn't immensely valuable |
I have |
Benchmark runs are scheduled for baseline = add2649 and contender = 866f1a1. 866f1a1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Draft untilWhich issue does this PR close?
re #2030
Rationale for this change
See #2030
What changes are included in this PR?
These are kept in different commits for easier review:
Follow on Items:
local::tests::bubble_up_io_errors
Are there any user-facing changes?
Not yet