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.

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

[HUDI-7367] Add makeQualified APIs #10607

Merged
merged 2 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
import org.apache.hudi.hadoop.fs.HoodieWrapperFileSystem;
import org.apache.hudi.hadoop.fs.NoOpConsistencyGuard;
import org.apache.hudi.metadata.HoodieTableMetadata;
import org.apache.hudi.storage.HoodieLocation;
import org.apache.hudi.storage.HoodieStorage;
import org.apache.hudi.storage.StorageSchemes;

import org.apache.hadoop.conf.Configuration;
Expand Down Expand Up @@ -122,6 +124,17 @@ public static Path makeQualified(FileSystem fs, Path path) {
return path.makeQualified(fs.getUri(), fs.getWorkingDirectory());
}

/**
* Makes location qualified with {@link HoodieStorage}'s URI.
*
* @param storage instance of {@link HoodieStorage}.
* @param location to be qualified.
* @return qualified location, prefixed with the URI of the target HoodieStorage object provided.
*/
public static HoodieLocation makeQualified(HoodieStorage storage, HoodieLocation location) {
return location.makeQualified(storage.getUri());
}

/**
* A write token uniquely identifies an attempt at one of the IOHandle operations (Merge/Create/Append).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
import org.apache.hudi.exception.HoodieException;
import org.apache.hudi.exception.HoodieIOException;
import org.apache.hudi.hadoop.fs.HadoopFSUtils;
import org.apache.hudi.hadoop.fs.HoodieWrapperFileSystem;
import org.apache.hudi.hadoop.fs.NoOpConsistencyGuard;
import org.apache.hudi.storage.HoodieLocation;
import org.apache.hudi.storage.HoodieStorage;
import org.apache.hudi.storage.hadoop.HoodieHadoopStorage;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
Expand Down Expand Up @@ -576,6 +581,22 @@ public void testGetFileExtension() {
assertThrows(NullPointerException.class, () -> FSUtils.getFileExtension(null));
}

@Test
public void testMakeQualified() {
FileSystem fs = HadoopFSUtils.getFs("file:///a/b/c", new Configuration());
FileSystem wrapperFs = new HoodieWrapperFileSystem(fs, new NoOpConsistencyGuard());
HoodieStorage storage = new HoodieHadoopStorage(fs);
HoodieStorage wrapperStorage = new HoodieHadoopStorage(wrapperFs);
assertEquals(new HoodieLocation("file:///x/y"),
FSUtils.makeQualified(storage, new HoodieLocation("/x/y")));
assertEquals(new HoodieLocation("file:///x/y"),
FSUtils.makeQualified(wrapperStorage, new HoodieLocation("/x/y")));
assertEquals(new HoodieLocation("s3://x/y"),
FSUtils.makeQualified(storage, new HoodieLocation("s3://x/y")));
assertEquals(new HoodieLocation("s3://x/y"),
FSUtils.makeQualified(wrapperStorage, new HoodieLocation("s3://x/y")));
}

private Path getHoodieTempDir() {
return new Path(baseUri.toString(), ".hoodie/.temp");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand All @@ -53,6 +54,11 @@ public String getScheme() {
return fs.getScheme();
}

@Override
public URI getUri() {
return fs.getUri();
}

@Override
public OutputStream create(HoodieLocation location, boolean overwrite) throws IOException {
return fs.create(convertHoodieLocationToPath(location), overwrite);
Expand Down
45 changes: 45 additions & 0 deletions hudi-io/src/main/java/org/apache/hudi/storage/HoodieLocation.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,51 @@ public URI toUri() {
return uri;
}

/**
* Returns a qualified location object.
*
* @param defaultUri if this location is missing the scheme or authority
* components, borrow them from this URI.
* @return this location if it contains a scheme and authority, or
* a new path that includes a path and authority and is fully qualified.
*/
@PublicAPIMethod(maturity = ApiMaturityLevel.EVOLVING)
public HoodieLocation makeQualified(URI defaultUri) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this ought to be a static method? I think we should instead make this a new constructor in HoodieLocation?

if (!isAbsolute()) {
throw new IllegalStateException("Only an absolute path can be made qualified");
}
HoodieLocation location = this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found this a bit weird. the assignment from this. is that needed?

URI locationUri = location.toUri();

String scheme = locationUri.getScheme();
String authority = locationUri.getAuthority();
String fragment = locationUri.getFragment();

if (scheme != null && (authority != null || defaultUri.getAuthority() == null)) {
return location;
}

if (scheme == null) {
scheme = defaultUri.getScheme();
}

if (authority == null) {
authority = defaultUri.getAuthority();
if (authority == null) {
authority = "";
}
}

URI newUri;
try {
newUri = new URI(scheme, authority,
normalize(locationUri.getPath(), true), null, fragment);
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
return new HoodieLocation(newUri);
}

@Override
public String toString() {
// This value could be overwritten concurrently and that's okay, since
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -51,6 +52,14 @@ public abstract class HoodieStorage implements Closeable {
@PublicAPIMethod(maturity = ApiMaturityLevel.EVOLVING)
public abstract String getScheme();

/**
* Returns a URI which identifies this HoodieStorage.
*
* @return the URI of this storage instance.
*/
@PublicAPIMethod(maturity = ApiMaturityLevel.EVOLVING)
public abstract URI getUri();

/**
* Creates an OutputStream at the indicated location.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,21 @@ public void testDepth() throws URISyntaxException {
assertEquals(4, new HoodieLocation(new HoodieLocation("s3://a/b/c"), "d/e").depth());
}

@Test
public void testMakeQualified() throws URISyntaxException {
URI defaultUri = new URI("hdfs://host1/dir1");
assertEquals(new HoodieLocation("hdfs://host1/a/b/c"),
new HoodieLocation("/a/b/c").makeQualified(defaultUri));
assertEquals(new HoodieLocation("hdfs://host2/a/b/c"),
new HoodieLocation("hdfs://host2/a/b/c").makeQualified(defaultUri));
assertEquals(new HoodieLocation("hdfs://host1/a/b/c"),
new HoodieLocation("hdfs:/a/b/c").makeQualified(defaultUri));
assertEquals(new HoodieLocation("s3://a/b/c"),
new HoodieLocation("s3://a/b/c/").makeQualified(defaultUri));
assertThrows(IllegalStateException.class,
() -> new HoodieLocation("a").makeQualified(defaultUri));
}

@Test
public void testEquals() {
assertEquals(new HoodieLocation("/foo"), new HoodieLocation("/foo"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Comparator;
Expand Down Expand Up @@ -99,6 +101,11 @@ public void testGetScheme() {
assertEquals("file", getHoodieStorage().getScheme());
}

@Test
public void testGetUri() throws URISyntaxException {
assertEquals(new URI("file:///"), getHoodieStorage().getUri());
}

@Test
public void testCreateWriteAndRead() throws IOException {
HoodieStorage storage = getHoodieStorage();
Expand Down
Loading