feat: change dataset uri to return full qualified url instead of object store path #2416

eddyxu · 2024-05-30T21:56:44Z

BREAKING CHANGE

github-actions · 2024-05-30T21:57:05Z

ACTION NEEDED

Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

codecov-commenter · 2024-05-30T22:11:02Z

Codecov Report

Attention: Patch coverage is 93.15068% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 79.65%. Comparing base (7153ddc) to head (2c4716c).

Files	Patch %	Lines
rust/lance-io/src/object_store.rs	91.30%	1 Missing and 1 partial ⚠️
rust/lance/src/dataset.rs	94.73%	0 Missing and 2 partials ⚠️
rust/lance/src/dataset/write.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2416      +/-   ##
==========================================
+ Coverage   79.51%   79.65%   +0.13%     
==========================================
  Files         205      205              
  Lines       56242    56285      +43     
  Branches    56242    56285      +43     
==========================================
+ Hits        44722    44833     +111     
+ Misses       8879     8801      -78     
- Partials     2641     2651      +10

Flag	Coverage Δ
unittests	`79.65% <93.15%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chebbyChefNEQ · 2024-05-31T01:34:11Z

rust/lance-index/src/vector/hnsw/builder.rs

@@ -790,7 +791,7 @@ mod tests {
        .unwrap();

        let object_store = ObjectStore::memory();
-        let path = object_store.base_path().child("test_builder_write_load");
+        let path = Path::from("test_builder_write_load");


why this change?

ObjectStore::base_path carries the same base path as Dataset::base. Logically, "base path" is not a attribute to ObjectStore which should be an handler for IO?

chebbyChefNEQ · 2024-05-31T01:34:51Z

rust/lance-io/src/object_store.rs

@@ -707,14 +696,6 @@ async fn configure_store(url: &str, options: ObjectStoreParams) -> Result<Object
        "s3" | "s3+ddb" => {
            storage_options.with_env_s3();

-            // if url.scheme() == "s3+ddb" && options.commit_handler.is_some() {


I think we should reenable this instead of delete it

Sure , will do

wjones127 · 2024-05-31T02:05:03Z

rust/lance/src/dataset.rs

+    pub fn uri(&self) -> &str {
+        &self.uri
    }


This is a breaking change to a public API. I've labelled this PR accordingly.

However, if we didn't want to make a breaking change, you could instead make a new method and deprecate this one.

Let's bump the version. The rust API does not have a large audience yet
https://crates.io/crates/lance/reverse_dependencies

This new release process does not allow us to accumulate a few breaking changes together for a new version?
https://github.com/lancedb/lance/blob/main/release_process.md#breaking-change-policy this is suggesting i manually bump the version, right?

This new release process does not allow us to accumulate a few breaking changes together for a new version?

You can accumulate them. Once you've bumped the version once, you don't have to do it again until the next release.

eddyxu added 2 commits May 30, 2024 14:53

simplify url

6724b57

az as well

45b41aa

github-actions bot added the chore label May 30, 2024

eddyxu changed the title ~~chore:~~ chore: change dataset uri to return full qualified url instead of object store path May 30, 2024

eddyxu added 7 commits May 30, 2024 16:06

a

6701fcd

buildable

b6bc624

fix

5e3e735

s

89735d3

a

8ded7c2

fix lint

28c27f3

add test

62fd17f

eddyxu marked this pull request as ready for review May 31, 2024 00:14

eddyxu requested review from westonpace, wjones127 and chebbyChefNEQ May 31, 2024 00:14

chebbyChefNEQ reviewed May 31, 2024

View reviewed changes

Merge branch 'main' into lei/dataset_uri

7d5904c

chebbyChefNEQ approved these changes May 31, 2024

View reviewed changes

revert change

99bad22

wjones127 changed the title ~~chore: change dataset uri to return full qualified url instead of object store path~~ feat: change dataset uri to return full qualified url instead of object store path May 31, 2024

wjones127 removed the chore label May 31, 2024

github-actions bot added enhancement New feature or request breaking-change labels May 31, 2024

wjones127 reviewed May 31, 2024

View reviewed changes

eddyxu added 3 commits May 30, 2024 19:43

v0.12

7b1b29d

one missing

ba67ec0

anything

2c4716c

eddyxu merged commit 0429aa7 into main May 31, 2024
19 checks passed

eddyxu deleted the lei/dataset_uri branch May 31, 2024 03:45

eddyxu mentioned this pull request May 31, 2024

feat(rust): expose Dataset uri in Rust #2410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: change dataset uri to return full qualified url instead of object store path #2416

feat: change dataset uri to return full qualified url instead of object store path #2416

eddyxu commented May 30, 2024 •

edited by wjones127

Loading

github-actions bot commented May 30, 2024

codecov-commenter commented May 30, 2024 •

edited

Loading

chebbyChefNEQ May 31, 2024

eddyxu May 31, 2024

chebbyChefNEQ May 31, 2024

eddyxu May 31, 2024

wjones127 May 31, 2024 •

edited

Loading

eddyxu May 31, 2024

wjones127 May 31, 2024

feat: change dataset uri to return full qualified url instead of object store path #2416

feat: change dataset uri to return full qualified url instead of object store path #2416

Conversation

eddyxu commented May 30, 2024 • edited by wjones127 Loading

github-actions bot commented May 30, 2024

codecov-commenter commented May 30, 2024 • edited Loading

Codecov Report

chebbyChefNEQ May 31, 2024

Choose a reason for hiding this comment

eddyxu May 31, 2024

Choose a reason for hiding this comment

chebbyChefNEQ May 31, 2024

Choose a reason for hiding this comment

eddyxu May 31, 2024

Choose a reason for hiding this comment

wjones127 May 31, 2024 • edited Loading

Choose a reason for hiding this comment

eddyxu May 31, 2024

Choose a reason for hiding this comment

wjones127 May 31, 2024

Choose a reason for hiding this comment

eddyxu commented May 30, 2024 •

edited by wjones127

Loading

codecov-commenter commented May 30, 2024 •

edited

Loading

wjones127 May 31, 2024 •

edited

Loading