-
Notifications
You must be signed in to change notification settings - Fork 3k
Add REST Compatibility Kit #10908
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
Add REST Compatibility Kit #10908
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,3 +40,66 @@ make generate | |
| ``` | ||
|
|
||
| The generated code is not being used in the project, but helps to see what the changes in the open-API definition are in the generated code. | ||
|
|
||
| # REST Compatibility Kit (RCK) | ||
|
|
||
| The REST Compatibility Kit (RCK) is a Technology Compatibility Kit (TCK) implementation for the | ||
| Iceberg REST Specification. This includes a series of tests based on the Java reference | ||
| implementation of the REST Catalog that can be executed against any REST server that implements the | ||
| spec. | ||
|
Comment on lines
+46
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe nit-picky, but I do believe RCK (TCK) ought to be based on the spec, not on the reference implementation. The RCK should pass on the reference impl.. but the foundation for it is the spec. Technically, the reference impl. may be incorrect with the respect to the spec (as any piece of software), so the test ought to validate that as well as catalog implementations from other projects.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree the intent is the RCK is based on the Spec and I don't feel this is saying otherwise. The purpose of the reference implementation is to faithfully implement the spec and the tests should validate that behavior. We're using the reference implementation tests as part of this because the "should" achieve the same objective. Creating a separate set of tests for the RCK would either be duplicative or imply that the reference implementation doesn't adhere to the spec (neither of which is good). |
||
|
|
||
| ## Test Configuration | ||
|
|
||
| The RCK can be configured using either environment variables or java system properties and allows | ||
| for configuring both the tests and the REST client. Environment variables prefixed by `CATALOG_` | ||
| are passed through the catalog configuring with the following mutations: | ||
|
|
||
| 1. The `CATALOG_` prefix is stripped from the key name | ||
| 2. Single underscore (`_`) is replaced with a dot (`.`) | ||
| 3. Double underscore (`__`) is replaced with a dash (`-`) | ||
| 4. The key names are converted to lowercase | ||
|
|
||
| A basic environment configuration would look like the following: | ||
|
|
||
| ```shell | ||
| CATALOG_URI=https://my_rest_server.io/ ## -> uri=https://my_rest_server.io/ | ||
| CATALOG_WAREHOUSE=test_warehouse ## -> warehouse=test_warehouse | ||
| CATALOG_IO__IMPL=org.apache.iceberg.aws.s3.S3FileIO ## -> io-impl=org.apache.iceberg.aws.s3.S3FileIO | ||
| CATALOG_CREDENTIAL=<oauth_key>:<oauth_secret> ## -> credential=<oauth_key>:<oauth_secret> | ||
| ``` | ||
|
|
||
| Java properties passed to the test must be prefixed with `rck.`, which can be used to configure some | ||
| test configurations described below and any catalog client properties. | ||
|
|
||
| An example of the same configuration using java system properties would look like the following: | ||
| ```shell | ||
| rck.uri=https://my_rest_server.io/ ## -> uri=https://my_rest_server.io/ | ||
| rck.warehouse=test_warehouse ## -> warehouse=test_warehouse | ||
| rck.io-impl=org.apache.iceberg.aws.s3.S3FileIO ## -> io-impl=org.apache.iceberg.aws.s3.S3FileIO | ||
| rck.credential=<oauth_key>:<oauth_secret> ## -> credential=<oauth_key>:<oauth_secret> | ||
| ``` | ||
|
|
||
| Some test behaviors are configurable depending on the catalog implementations. Not all behaviors | ||
| are strictly defined by the REST Specification. The following are currently configurable: | ||
|
|
||
| | config | default | | ||
| |-------------------------------|---------| | ||
| | rck.requires-namespace-create | true | | ||
| | rck.supports-serverside-retry | true | | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| ## Running Compatibility Tests | ||
|
|
||
| The compatibility tests can be invoked via gradle with the following: | ||
|
|
||
| Note: The default behavior is to run a local http server with a jdbc backend for testing purposes, | ||
| so `-Drck.local=false` must be set to point to an external REST server. | ||
|
|
||
| ```shell | ||
| ./gradlew :iceberg-open-api:test --tests RESTCompatibilityKitSuite \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add an example or instructions for running the RCK in a downstream Catalog project?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I fully follow, but I would think the RCK is run against a downstream catalog project, not necessarily in one. I suppose you could subclass this suite and include that as part of the tests, but I think that probably is a better follow up once someone has a reference example integrated that we can use to validate an approach. |
||
| -Drck.local=false \ | ||
| -Drck.requires-namespace-create=true \ | ||
| -Drck.uri=https://my_rest_server.io/ \ | ||
| -Drck.warehouse=test_warehouse \ | ||
| -Drck.credential=<oauth_key>:<oauth_secret> | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| /* | ||
| * 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.iceberg.rest; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import org.apache.iceberg.catalog.CatalogTests; | ||
| import org.apache.iceberg.util.PropertyUtil; | ||
| import org.junit.jupiter.api.AfterAll; | ||
| import org.junit.jupiter.api.BeforeAll; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @ExtendWith(RESTServerExtension.class) | ||
| public class RESTCompatibilityKitCatalogTests extends CatalogTests<RESTCatalog> { | ||
| private static final Logger LOG = LoggerFactory.getLogger(RESTCompatibilityKitCatalogTests.class); | ||
|
|
||
| private static RESTCatalog restCatalog; | ||
|
|
||
| @BeforeAll | ||
| static void beforeClass() throws Exception { | ||
| restCatalog = RCKUtils.initCatalogClient(); | ||
|
|
||
| assertThat(restCatalog.listNamespaces()) | ||
| .withFailMessage("Namespaces list should not contain: %s", RCKUtils.TEST_NAMESPACES) | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .doesNotContainAnyElementsOf(RCKUtils.TEST_NAMESPACES); | ||
| } | ||
|
|
||
| @BeforeEach | ||
| void before() { | ||
| try { | ||
| RCKUtils.purgeCatalogTestEntries(restCatalog); | ||
| } catch (Exception e) { | ||
| LOG.warn("Failure during test setup", e); | ||
| } | ||
| } | ||
|
|
||
| @AfterAll | ||
| static void afterClass() throws Exception { | ||
| restCatalog.close(); | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| protected RESTCatalog catalog() { | ||
| return restCatalog; | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean requiresNamespaceCreate() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), | ||
| RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE, | ||
| super.requiresNamespaceCreate()); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean supportsServerSideRetry() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), RESTCompatibilityKitSuite.RCK_SUPPORTS_SERVERSIDE_RETRY, true); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean overridesRequestedLocation() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), | ||
| RESTCompatibilityKitSuite.RCK_OVERRIDES_REQUESTED_LOCATION, | ||
| false); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /* | ||
| * 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.iceberg.rest; | ||
|
|
||
| import org.junit.platform.suite.api.SelectClasses; | ||
| import org.junit.platform.suite.api.Suite; | ||
| import org.junit.platform.suite.api.SuiteDisplayName; | ||
|
|
||
| /** | ||
| * Iceberg REST Compatibility Kit | ||
| * | ||
| * <p>This test suite provides the ability to run the Iceberg catalog tests against a remote REST | ||
| * catalog implementation to verify the behaviors against the reference implementation catalog | ||
| * tests. | ||
| * | ||
| * <p>The tests can be configured through environment variables or system properties. By default, | ||
| * the tests will run using a local http server using a servlet implementation that leverages the | ||
| * {@link RESTCatalogAdapter}. | ||
| */ | ||
| @Suite | ||
| @SuiteDisplayName("Iceberg REST Compatibility Kit") | ||
| @SelectClasses({RESTCompatibilityKitCatalogTests.class, RESTCompatibilityKitViewCatalogTests.class}) | ||
| public class RESTCompatibilityKitSuite { | ||
| static final String RCK_REQUIRES_NAMESPACE_CREATE = "rck.requires-namespace-create"; | ||
| static final String RCK_SUPPORTS_SERVERSIDE_RETRY = "rck.supports-serverside-retry"; | ||
| static final String RCK_OVERRIDES_REQUESTED_LOCATION = "rck.overrides-requested-location"; | ||
|
|
||
| protected RESTCompatibilityKitSuite() {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| /* | ||
| * 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.iceberg.rest; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import org.apache.iceberg.catalog.Catalog; | ||
| import org.apache.iceberg.util.PropertyUtil; | ||
| import org.apache.iceberg.view.ViewCatalogTests; | ||
| import org.junit.jupiter.api.AfterAll; | ||
| import org.junit.jupiter.api.BeforeAll; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @ExtendWith(RESTServerExtension.class) | ||
| public class RESTCompatibilityKitViewCatalogTests extends ViewCatalogTests<RESTCatalog> { | ||
| private static final Logger LOG = | ||
| LoggerFactory.getLogger(RESTCompatibilityKitViewCatalogTests.class); | ||
| private static RESTCatalog restCatalog; | ||
|
|
||
| @BeforeAll | ||
| static void beforeClass() throws Exception { | ||
| restCatalog = RCKUtils.initCatalogClient(); | ||
|
|
||
| assertThat(restCatalog.listNamespaces()) | ||
| .withFailMessage("Namespaces list should not contain: %s", RCKUtils.TEST_NAMESPACES) | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .doesNotContainAnyElementsOf(RCKUtils.TEST_NAMESPACES); | ||
| } | ||
|
|
||
| @BeforeEach | ||
| void before() { | ||
| try { | ||
| RCKUtils.purgeCatalogTestEntries(restCatalog); | ||
| } catch (Exception e) { | ||
| LOG.warn("Failure during test setup", e); | ||
| } | ||
| } | ||
|
|
||
| @AfterAll | ||
| static void afterClass() throws Exception { | ||
| restCatalog.close(); | ||
| } | ||
|
|
||
| @Override | ||
| protected RESTCatalog catalog() { | ||
| return restCatalog; | ||
| } | ||
|
|
||
| @Override | ||
| protected Catalog tableCatalog() { | ||
| return restCatalog; | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean requiresNamespaceCreate() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE, true); | ||
danielcweeks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| protected boolean supportsServerSideRetry() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), RESTCompatibilityKitSuite.RCK_SUPPORTS_SERVERSIDE_RETRY, true); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean overridesRequestedLocation() { | ||
| return PropertyUtil.propertyAsBoolean( | ||
| restCatalog.properties(), | ||
| RESTCompatibilityKitSuite.RCK_OVERRIDES_REQUESTED_LOCATION, | ||
| false); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.