You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "eric-maynard (via GitHub)" <gi...@apache.org> on 2024/10/08 17:57:43 UTC
[PR] Add a FileIO implementation for WASB [polaris]
eric-maynard opened a new pull request, #360:
URL: https://github.com/apache/polaris/pull/360
# Description
In #344, checks within Polaris were relaxed so that Azure [WASB](https://learn.microsoft.com/en-us/azure/storage/blobs/data-lake-storage-abfs-driver#prior-capability-the-windows-azure-storage-blob-driver) paths can be used to e.g. create a catalog, and so that WASB paths and ABFS paths that referred to the same object could be compared. This PR introduces a FileIO implementation that Polaris can use to interact with entities that use such a WASB path. The FileIO implementation simply rewrites WASB paths into ABFS paths, and delegations interactions with the ABFS paths to the Iceberg [ADLSFileIO](https://iceberg.apache.org/javadoc/1.5.2/org/apache/iceberg/azure/adlsv2/ADLSFileIO.html).
## Type of change
Please delete options that are not relevant.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation update
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
# How Has This Been Tested?
To use the new FileIO easily in Polaris, you can change the `factoryType` in the server config to `wasb`.
With this change, the table creation API succeeds:
```
curl -X POST "http://localhost:8181/api/catalog/v1/wasb_test/namespaces/wasb_test_namespace/tables" \
> -H "Authorization: Bearer ${token}" \
> -H "Content-Type: application/json" \
> -H "X-Iceberg-Access-Delegation: vended-credentials" \
> -d '{
> "name": "manual-table-1",
> "schema": {
> "type": "struct",
> "fields": [
> {"id": 101, "name": "id", "type": "string", "required": false}
> ]
> }
> }' \
> 2> /dev/null && echo
{"metadata-location":"abfss://....","metadata":{"format-version":2,"table-uuid":"...","location":"wasbs://...", ...}}
```
Without this FileIO, it will fail with an error such as:
```
{"error":{"message":"Invalid ADLS URI: wasbs://...","type":"ValidationException","code":400}}
```
# Checklist:
Please delete options that are not relevant.
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1793915018
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.io.FileIO;
+
+/** A {@link FileIOFactory} that translates WASB paths to ABFS ones */
+@JsonTypeName("wasb")
+public class WasbTranslatingFileIOFactory implements FileIOFactory {
+ @Override
+ public FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
Review Comment:
It won't work exactly out of the box, since FileIOFactory doesn't have a way to wrap a FileIO today. So it's not clear how the `chain` implementation would actually chain them. In [this feature branch](https://github.com/apache/polaris/compare/main...eric-maynard:polaris:delegating-io?expand=1) I toyed with adding a new interface called `SupportsFileIOWrapping` for this purpose, but I don't know that this is necessary just to get WASB working E2E
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1792877341
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
> it looks like the current structure of FileIOFactory is forcing this to wrap all FileIOs including non-Azure-related ones
It's doing a little more than just scheme translating, and this is only true if Polaris is actually configured by an admin to use this factory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1792730325
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
Oh I see you also have the endpoint rewrite from `blob` to `dfs` which wouldn't fit into that. I agree we might not want to go down the road of overly general regexes either.
Per https://github.com/apache/iceberg/issues/10127#issuecomment-2080302481 the underlying Azure SDK doesn't actually seem to actually care about the endpoint and will internally sort into both a `dfs` and a `blob` client, so in theory a scheme-only replacement should still work, but I guess it could be more fragile.
@collado-mike What do you think? Should we just keep this very wasb-specific?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1792553886
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.io.FileIO;
+
+/** A {@link FileIOFactory} that translates WASB paths to ABFS ones */
+@JsonTypeName("wasb")
+public class WasbTranslatingFileIOFactory implements FileIOFactory {
+ @Override
+ public FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
Review Comment:
At this point it's starting to seem that we need a way to do a chain of delegation. Both this class (or the suggested generalized `SchemeTranslatingFileIOFactory`) as well as `MeasuredFileIOFactory` fundamentally delegators, so maybe we should've entrenched an interface for delegation instead of just loading from `String ioImpl`.
If we make the delegation behavior configured as construction-time params or injection then we don't have to change the method signatures or callsites either. Basically like this:
mainFileIOFactory = new SchemeTranslatingFileIOFactory(
new MeasuredFileIOFactory(new DefaultFileIOFactory()),
Map.of("wasb", "abfs", "wasbs", "abfss"));
And then we get rid of the hard-coded `CatalogUtil.loadFileIO` in both `SchemeTranslatingFileIOFactory` and `MeasuredFileIOFactory`:
@Override
public FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
return new SchemeTranslatingFileIO(this.delegateFactory.loadFileIO(ioImpl, properties);
}
The drawback I guess is the config for setting the delegate factory is specific to each outer delegator factory:
io:
factoryType: scheme-translating
schemeTranslatingDelegateType: measured
measuredFactoryDelegateType: default
But this might be better than forcing delegation through a single config syntax since delegation might not be consistent for all delegator types. For example you might have something like:
multiplexingIoFactoryDelegateTypes: r2,s3n,viewfs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1792520150
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
Since it looks like the current structure of FileIOFactory is forcing this to wrap all FileIOs including non-Azure-related ones, we might as well make the class itself more generalized and we could move the Azure-specificity into config.
What if we just called this thing `SchemeTranslatingFileIO` that takes a map of source schemes to destination schemes? And get the map through config
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
+ this.io = io;
+ }
+
+ private static String translate(String path) {
+ if (path == null) {
+ return null;
+ } else {
+ StorageLocation storageLocation = StorageLocation.of(path);
+ if (storageLocation instanceof AzureLocation azureLocation) {
+ String scheme = azureLocation.getScheme();
+ if (scheme.startsWith(WASB_SCHEME)) {
+ scheme = scheme.replaceAll(WASB_SCHEME, ABFS_SCHEME);
Review Comment:
Though it *shouldn't* matter, `replaceFirst` might convey intent a bit better, but ideally we can just be more explicit with a `Map<String, String>` lookup anyways -- being explicit about both `wasb -> abfs` and `wasbs -> abfss` in this case is probably worth the extra boilerplate even if we don't get to "save" on handling the "optional" "s" suffix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1793904244
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
If I were designing this, I would have a `TransformingFileIO` with a constructor like
```java
public TransformingFileIO(Function<String, String> pathTransformer, FileIOFactory delegate) {
}
```
and the azure class would extend that class and pass in a function that did the wasb->abfs conversion. But that's the kind of code refactoring that can be done later without any compatibility or behavior impact, so... 🤷🏽♂️
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.io.FileIO;
+
+/** A {@link FileIOFactory} that translates WASB paths to ABFS ones */
+@JsonTypeName("wasb")
+public class WasbTranslatingFileIOFactory implements FileIOFactory {
+ @Override
+ public FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
Review Comment:
I don't think we can avoid working on this now. A production use case will require things like metering, throttling, as well as azure support. As is, the configuration only supports one IO factory, meaning you couldn't get all three without writing a new class that does all three.
I think the configuration would work fine as
```
io:
factoryType: chain
delegates:
- measured
- wasb
- default
```
I think dropwizard should support detecting the generic type argument in something like
```
public void setDelegates(List<FileIOFactory> delegates);
```
so the above configuration should work out of the box.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1793948411
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
I really like this idea and actually @dennishuo and I had discussed something similar.
My concern for the time being is that it's unclear how we can pass in that `pathTransformer` via the YAML config file, and that I don't necessarily want to put designing a syntax for arbitrary string transformation onto the critical path for making WASB work.
Agreed that this makes a lot of sense as something to design and follow up with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1793948411
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIO.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import java.util.Map;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/**
+ * A {@link FileIO} implementation that translates WASB paths into ABFS paths and then delegates to
+ * another underlying FileIO implementation
+ */
+public class WasbTranslatingFileIO implements FileIO {
+ private final FileIO io;
+
+ private static final String WASB_SCHEME = "wasb";
+ private static final String ABFS_SCHEME = "abfs";
+
+ public WasbTranslatingFileIO(FileIO io) {
Review Comment:
I really like this idea and actually @dennishuo and I had discussed something similar.
My concern for the time being is that it's unclear how we can pass in that `pathTransformer` via the YAML config file, and that I don't necessarily want to put designing a syntax for arbitrary string transformation onto the critical path for making WASB work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #360:
URL: https://github.com/apache/polaris/pull/360#discussion_r1792579231
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.catalog.io;
+
+import com.fasterxml.jackson.annotation.JsonTypeName;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.io.FileIO;
+
+/** A {@link FileIOFactory} that translates WASB paths to ABFS ones */
+@JsonTypeName("wasb")
+public class WasbTranslatingFileIOFactory implements FileIOFactory {
+ @Override
+ public FileIO loadFileIO(String ioImpl, Map<String, String> properties) {
Review Comment:
I can see how it could be a bigger undertaking to properly entrench how we want to convey such a delegation chain though, so I don't feel too strongly about whether to tackle this in this PR or in a later followup
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add a FileIO implementation for WASB [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike merged PR #360:
URL: https://github.com/apache/polaris/pull/360
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org