[WIP] REST Based Catalog #3424

kbendick · 2021-10-29T23:22:51Z

There are many users who are coming to the data space for the first time, or otherwise aren't used to used HMS.

For many of them, implementing a REST server and then choosing how to handle things on the server side is likely their preferred method. This would also allow people to plug in their own implementation around a somewhat known interface.

I'm opening this up for comment as I've begun some work on a REST catalog API, as well as an example implementation.

I'm seeking feedback and will also have a proposal eventually after it's been discussed in the community a bit. 🙂

api/src/main/java/org/apache/iceberg/exceptions/AuthorizationDeniedException.java

rdblue · 2021-10-31T21:12:32Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+
+  }
+
+  private CreateNamespaceRequest(String namespaceName, Map<String, String> properties) {


The Catalog API allows a namespace that is any number of levels. Should this be Namespace, List<String>, or String[]?

It makes sense to use Namespace to me, but in the JSON, I'd prefer it to be a List<String> or String[] so users can specify as many levels as they'd like.

Will update.

I will come back and update these things after we discuss the proposed specification 👍

rdblue · 2021-10-31T21:13:57Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+ */
+public class CreateNamespaceRequest implements Serializable {
+
+  // TODO - Use protected so users can extend this for their own impls. Or an interface.


Are user implementations to be supported? I was thinking that we would treat the REST API as a spec so it wouldn't necessarily make sense to allow customization.

You're right. I actually meant more what you said (about it as a spec). Bad phrasing on my part. I have been working through various versions and different code pathways, and ultimately decided to introduce this initially with CreateNamespaceRequest to open discussion.

I'll update this to be more clear that this designed to be a spec.

rdblue · 2021-10-31T21:15:13Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceResponse.java

+ */
+public class CreateNamespaceResponse {
+  private Namespace namespace;
+  private Map<String, String> properties;


Are these the properties that the namespace has after create? For example, this may include a "created-at" property that was added by the server automatically.

Yeah that was my thinking (about server side additions). I was thinking that this would be either:

Properties the server added, such as created-at.

The entire properties of the namespace, including the ones the user provided, to give a full picture.

Yeah, I meant #2. All properties of the namespace, including the ones the user sent and the ones that were added automatically. We should document this in the class Javadoc.

rdblue · 2021-10-31T21:15:46Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+  }
+
+  void setNamespaceName(String name) {
+    this.namespaceName = name;


Why include setters and a builder? Should this be immutable?

I included setters and getters so that it would be a proper bean for serialization purposes.

I admittedly don't love them. Going through, I see that the Aliyun is using springboot and annotations instead directly in their code.

Open to suggestions / ideas around that. And possibly this ultimately won't wind up needing to be a bean at all (not a huge fan of beans in general though I recognize they have their time and place - though that might not be now).

Ah, looks like this is probably needed because there is no serializer and it needs to adhere to the bean interface.

I think this depends on how we want to specify the protocol. I prefer documenting the JSON objects that will be sent, which probably means going through and creating serializers/deserializers for each object.

Yeah. I also prefer documenting the JSON objects. I had thought that the getters and setters were still needed for the serializers though.

But I'm relatively green with the Apache HTTP library that I chose for the reference calls, so maybe I can get rid of this.

I'll play around with it, but definitely agreed that I prefer to specify the protocol and to keep it language agnostic (e.g. so people can use another language on the server side in addition to the clarity that documenting the JSON objects brings over the Java classes).

Admittedly, as I come from more of a Scala background (partcularly when implementing REST API clients), in some places I'm not sure what needs to be a bean and what doesn't for serialization. I've typically relied on case classes, which can pretty effortlessly be converted to and from JSON with much work, or more modern over the network serialization standards such as protobuf, thrift, or even Avro.

So some things might presently be beans that don't need to be. I'm happy to clean that up as I go along and come to be more familiar with some of these java bean related things =)

rdblue · 2021-10-31T21:18:15Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+  }
+
+  @Override
+  protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {


I'm not sure that we want to implement BaseMetastoreCatalog, since it has conventions that we don't need, like this method. This method is a way for the catalog to determine the default location for a table. But shouldn't this be done on the server side for a REST-based catalog? That makes the most sense to me because we want to keep the client as simple as possible and do all of the customization in the server that is easier to change. Deployed client Jars are hard to update!

That makes sense to me. For the REST catalog, especially as a spec, the server should have a bit more control over what's done. I had a version that didn't use BaseMetastoreCatalog (and still do), but decided to forgo it initially given the deviation from the norm.

I'll work on switching it back as I'm fleshing out ideas for the spec.

rdblue · 2021-10-31T21:19:07Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+        .withProperties(metadata)
+        .build();
+
+    String path = properties.getOrDefault("create-namespace-path", "databases");


Does the default here need a starting /?

Hmm. I don't think it does, but I will add in a check.

Even if it doesn't, it seems a bit weird to have a path that doesn't start with a /. So I'll add some normalization code (or use some from elsewhere) to allow for that.

rdblue · 2021-10-31T21:22:07Z

core/src/main/java/org/apache/iceberg/rest/RestClient.java

+import java.io.Closeable;
+
+/**
+ * Eventual Interface for pluggable RestClients.


Do you mean to allow customizing the HTTP client implementation?

Yes. That's what I mean. People might have a preference on the HTTP client implementation. Particularly if they use HTTP2 or HTTP3 even. So long as it conforms to the spec, it should in theory be fine.

But that was my thinking.

I think we can actually learn a bit from the HTTP client in AWS SDK v2, it supports plugging in any HTTP client implementation. We can borrow the same design pattern here.

core/src/main/java/org/apache/iceberg/rest/RestException.java

rdblue · 2021-10-31T21:24:10Z

core/src/main/java/org/apache/iceberg/rest/UncheckedRestException.java

+ * Exception thrown by the REST client and catalog, to wrap a checked exception
+ * in an unchecked exception.
+ */
+public class UncheckedRestException extends RuntimeException {


Can't the other rest exception handle checked exceptions?

Absolutely. Good catch. I had a few different branches where I was working out some possible ideas, and somehow I managed to leave both of those in.

I'll remove this one. 👍

rdblue · 2021-10-31T21:46:30Z

core/src/main/java/org/apache/iceberg/rest/http/ErrorHandlers.java

+    };
+  }
+
+  static String getResponseBody(CloseableHttpResponse response) {


Can this be private? Also, try not to use get in names. If you can't replace it with something more specific (like find, fetch, create, etc.) then it's likely that it is just a filler word that can be omitted.

rdblue · 2021-10-31T21:49:30Z

core/src/main/java/org/apache/iceberg/rest/http/ErrorHandlers.java

+      icebergException = response.getHeader("X-Iceberg-Exception").getValue();
+    } catch (Exception e) {
+      // TODO - Better error message and handling. Will be refactoring this anyway.
+      LOG.error("Encountered an error when getting the X-Iceberg-Exception header", e);


Instead of using a header, what about having a way to encode an exception in the JSON payload? That seems to be the approach outlined here: https://www.baeldung.com/rest-api-error-handling-best-practices

Agreed. That's more in line with how I've previously done it as well. The Facebook example is most in line with what I've previously used, where there's a dedicted error payload in the response body and then possibly even further error codes beyond just the normal 4xx, and those error codes map to what happened.

{ "error": { "message": "Missing Bearer header.", "type": "OAuthException", "code": 40101, // We document what 401xx error codes correspond to as part of the spec "trace_id": "AWswcVwbcqfgrSgjG80MtqJ" }, data: { // Successful response payload } }

I will refactor to use an approach like that.

+1. I'm not sure that we need error codes, but we can always add them if people find them useful.

rdblue · 2021-10-31T21:53:02Z

core/src/main/java/org/apache/iceberg/rest/http/RequestResponseSerializers.java

+  public static class NamespaceSerializer extends JsonSerializer<Namespace> {
+    @Override
+    public void serialize(Namespace namespace, JsonGenerator gen, SerializerProvider serializers) throws IOException {
+      gen.writeString(namespace.toString());


I think this should probably be a list of strings to avoid losing the hierarchy when . is in one of the namespace identifiers.

Yeah agreed. Let's sort it out in the spec for a bit and then make the necessary updates to the sereializers.

As mentioned in the community sync up, given that Trino needs some kind of JSON representation, I'm also going to meet with Jack to discuss and get his expertise on it too. 🙂

rdblue · 2021-10-31T21:54:08Z

core/src/main/java/org/apache/iceberg/rest/http/RequestResponseSerializers.java

+
+  public static class NamespaceDeserializer extends JsonDeserializer<Namespace> {
+    @Override
+    public Namespace deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {


Can you update these to use full word names? It's generally more readable to use parser rather than p. Also, I don't think that it is any easier to type ctxt than to type context, and it is definitely less readable.

100% agreed. After rebasing, I encountered a lot of HiddenField checkstyle errors and probably went a little overboard on changing the names.

rdblue · 2021-10-31T21:55:49Z

core/src/main/java/org/apache/iceberg/rest/http/RequestResponseSerializers.java

+  public static class SchemaSerializer extends JsonSerializer<Schema> {
+    @Override
+    public void serialize(Schema schema, JsonGenerator gen, SerializerProvider serializers)
+        throws IOException {


Does this actually throw IOException? If not, can we remove it?

Yeah this does throw IOException. Specifically, SchemaParser.toJson does.

I can wrap it with the UncheckedIOException or something else though.

Actually, my mistake. The serialize method itself declares the IOException. It looks like there's a delegate that can be used, so either I'll figure out the existing delegate or wrap with my own to avoid all of the checked exceptions if possible.

rdblue · 2021-10-31T21:58:16Z

core/src/main/java/org/apache/iceberg/rest/http/RequestResponseSerializers.java

+    public SortOrder deserialize(JsonParser p, DeserializationContext context)
+        throws IOException {
+      JsonNode jsonNode = p.getCodec().readTree(p);
+      Schema schema = (Schema) context.getAttribute("schema");


What writes the schema into the sort order? It looks like these are going to require some context.

The SortOrderParser.fromJson(Schema schema, JsonNode node) call that returns the final SortOrder is what writes the schema into it.

Though I do feel it could be made much easier to grok.

That method constructs a SortOrder for a given schema that must be passed in. It is on the deserialization side, not the serialization side. For the write side, SortOrderParser.toJson produces JSON but doesn't write the schema into it.

kbendick

Thanks for the feedback Ryan. I will use it to iterate on this. As mentioned, I had a few different ideas for the spec that I was working on, so I will clean up and then add in some of those as well.

kbendick · 2021-10-30T01:55:55Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+  String getNamespaceName() {
+    return namespaceName;
+  }
+
+  void setNamespaceName(String name) {
+    this.namespaceName = name;
+  }


These are beans for serialization purposes (typically we avoid the word get in methods as usually there's a more descriptive verb).

There's code gen tools, i.e. lombock, but for something that is to be used as part of an AP / spec, it might be wise to avoid that if possible. Very open to suggestions on that idea.

kbendick · 2021-10-31T21:46:07Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+ */
+public class CreateNamespaceRequest implements Serializable {
+
+  // TODO - Use protected so users can extend this for their own impls. Or an interface.


You're right. I actually meant more what you said (about it as a spec). Bad phrasing on my part. I have been working through various versions and different code pathways, and ultimately decided to introduce this initially with CreateNamespaceRequest to open discussion.

I'll update this to be more clear that this designed to be a spec.

kbendick · 2021-10-31T21:49:44Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+  }
+
+  void setNamespaceName(String name) {
+    this.namespaceName = name;


I included setters and getters so that it would be a proper bean for serialization purposes.

I admittedly don't love them. Going through, I see that the Aliyun is using springboot and annotations instead directly in their code.

Open to suggestions / ideas around that. And possibly this ultimately won't wind up needing to be a bean at all (not a huge fan of beans in general though I recognize they have their time and place - though that might not be now).

kbendick · 2021-10-31T21:51:21Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceResponse.java

+ */
+public class CreateNamespaceResponse {
+  private Namespace namespace;
+  private Map<String, String> properties;


Yeah that was my thinking (about server side additions). I was thinking that this would be either:

Properties the server added, such as created-at.

The entire properties of the namespace, including the ones the user provided, to give a full picture.

kbendick · 2021-10-31T21:53:34Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+  }
+
+  @Override
+  protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {


That makes sense to me. For the REST catalog, especially as a spec, the server should have a bit more control over what's done. I had a version that didn't use BaseMetastoreCatalog (and still do), but decided to forgo it initially given the deviation from the norm.

I'll work on switching it back as I'm fleshing out ideas for the spec.

kbendick · 2021-10-31T21:54:57Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+        .withProperties(metadata)
+        .build();
+
+    String path = properties.getOrDefault("create-namespace-path", "databases");


Hmm. I don't think it does, but I will add in a check.

Even if it doesn't, it seems a bit weird to have a path that doesn't start with a /. So I'll add some normalization code (or use some from elsewhere) to allow for that.

kbendick · 2021-10-31T21:56:19Z

core/src/main/java/org/apache/iceberg/rest/RestClient.java

+import java.io.Closeable;
+
+/**
+ * Eventual Interface for pluggable RestClients.


Yes. That's what I mean. People might have a preference on the HTTP client implementation. Particularly if they use HTTP2 or HTTP3 even. So long as it conforms to the spec, it should in theory be fine.

But that was my thinking.

core/src/main/java/org/apache/iceberg/rest/RestException.java

kbendick · 2021-10-31T21:58:13Z

core/src/main/java/org/apache/iceberg/rest/UncheckedRestException.java

+ * Exception thrown by the REST client and catalog, to wrap a checked exception
+ * in an unchecked exception.
+ */
+public class UncheckedRestException extends RuntimeException {


Absolutely. Good catch. I had a few different branches where I was working out some possible ideas, and somehow I managed to leave both of those in.

I'll remove this one. 👍

kbendick · 2021-10-31T22:02:18Z

core/src/main/java/org/apache/iceberg/rest/http/RequestResponseSerializers.java

+    }
+  }
+
+  public static class TableMetadataSerializer extends JsonSerializer<TableMetadata> {


We'll need a deserializer as well. Currently that depends on having a metadata file. I have a few ideas around this and will push something during my next round of clean up on this. Possibly placing it in a separate PR as well.

Eventually, I plan on breaking this into smaller PRs to be more easily digested, but this is to provide a larger picture overview.

Yeah, there's no need to only read from a metadata file.

rdblue · 2021-10-31T22:02:22Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+  public <T> T execute(
+      Method method, String path, Object body, Class<T> responseType,
+      Consumer<CloseableHttpResponse> errorHandler) {
+    HttpUriRequestBase request = new HttpUriRequestBase(method.name(), URI.create(String.format("%s/%s", baseUrl,


Style: Prefer line wrapping as high in the parse tree as possible. Here, wrapping within the String.format within URI.create within new HttpUriRequestBase makes it hard to distinguish what level path is an argument to. Instead, wrapping each arg to a new line or just adding a newline for URI.create(...) is more readable.

rdblue · 2021-10-31T22:03:19Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+      try {
+        request.setEntity(new StringEntity(mapper.writeValueAsString(body)));
+      } catch (JsonProcessingException e) {
+        throw new UncheckedRestException(e, "Failed to write request body");


I think it would be good to have an exception that distinguishes problems before sending the request, like this one.

rdblue · 2021-10-31T22:04:05Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+
+    try (CloseableHttpResponse response = httpClient.execute(request)) {
+      if (response.getCode() != HttpStatus.SC_OK) {
+        errorHandler.accept(response);


Should this return after handling the response in case it doesn't throw an exception?

+1. I decided to use the Apache HTTP library, as we're in an Apache project. But I'm more familiar with other HTTP libraries. I will return here and then consider more about libraries etc. I'm a big fan of OkHTTP, but I recognize that might not be in-line with the fact that this is an Apache repo.

We can use whatever we want as long as the license is compatible. That said, if the library is already included with Spark and Flink, we generally prefer to use it so we don't need to shade and relocate it. That's why Apache HTTP is most often used.

rdblue · 2021-10-31T22:04:44Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+      HttpEntity entity = response.getEntity();
+      return mapper.readValue(entity.getContent(), responseType);
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);


What throws the IOException? Can we add context about what failed here?

Yeah I can add context. Currently, I believe it's either mapper.readValue or response.getEntity that throws it.

I'll double check and then add a note. I can also wrap and throw individually if multiple things are throwing it.

rdblue · 2021-10-31T22:04:56Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+      return mapper.readValue(entity.getContent(), responseType);
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    } catch (Exception e) {


Something throws Exception?

rdblue · 2021-10-31T22:05:29Z

core/src/main/java/org/apache/iceberg/rest/http/HttpClient.java

+    private Consumer<CloseableHttpResponse> defaultErrorHandler = ErrorHandlers.databaseErrorHandler();
+
+    private Builder() {
+


Style: no blank line in empty methods.

rdblue · 2021-10-31T22:07:40Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+  private Configuration hadoopConf;
+  private FileIO fileIO;
+
+  // TODO - Refactor to use interface, which doesn't currently have POST etc embedded into it.


Yeah, it seems like you already know enough about the methods in HttpClient to fill out the interface.

rdblue · 2021-10-31T22:13:01Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+/**
+ * Represents a REST request to create a namespace / database.
+ */
+public class CreateNamespaceRequest implements Serializable {


Why Serializable? Is this needed for the JSON bean serialization?

kbendick · 2021-11-15T06:11:37Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceRequest.java

+ * Represents a REST request to create a namespace / database.
+ *
+ * Possible example POST body (outer format object possibly only used on Response):
+ *   { "data": { "namespace": ["prod", "accounting"], properties: {} }}


Currently it's actually as follows, without using an array for Namespace.

{ "namespace": "prod.accounting", "properties": { "key1": "value1" } }

Now that I have the mock server going, I'm going to enter an OpenAPI spec and then run the tests over the spec (as I've done for a create namespace based one).

Will put the spec in this PR as a separate document.

…tation

…uld be closed by the catalog

… from other branches

…a mocks

jackye1995 · 2021-11-17T19:44:17Z

api/src/main/java/org/apache/iceberg/rest/CreateNamespaceResponse.java

+ * structure, including the possibility of richer metadata on errors, such as tracing telemetry or follw
+ * up user instruction.
+ *
+ * {


I think you need <pre> HTML tag for these code blocks to show properly in javadoc.

jackye1995 · 2021-11-17T19:47:30Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+
+    // TODO - Possibly authenticate with the server initially and then have the server return some of this information
+    Preconditions.checkNotNull(
+        properties.getOrDefault("baseUrl", null),


would it be possible to reuse CatalogProperties.URI for this?

Oh that's a good call. Will update.

jackye1995 · 2021-11-17T19:50:07Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+    this.baseUrl = properties.get("baseUrl");
+    this.fileIO = initializeFileIO(properties);
+
+    this.mapper = new ObjectMapper();


I think it is recommended to use object mapper as a singleton. We can reuse JsonUtil.mapper() and add util methods around that to register modules.

And actually why not directly register the serdes in that mapper directly, instead of only registering it when initializing the REST catalog? I think it does not hurt to do that even for people not using REST catalog.

That would certainly make it simpler. And I don't see the harm in registering the serdes directly, which would make a lot of things simpler. Good suggestion. 👍

jackye1995 · 2021-11-17T19:54:08Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RestCatalog extends BaseMetastoreCatalog implements Closeable, SupportsNamespaces, Configurable {


I think it's probably better to directly implement it from interface instead of extending the base implementation. All the calls are delegated to the server anyway, there is not much benefit I see in inheriting the base logic. Some things like default location are really just util methods but not formalized idea that should not be inherited for REST catalog implementation.

Yeah I am in agreement with that, BaseMetastoreCatalog doesn't provide much value as mentioned.

I stepped away from this PR for a bit to just focus on fleshing out the spec in the other PR. I will refactor to remove BaseMetastoreCatalog from here.

jackye1995 · 2021-11-17T19:55:14Z

core/src/main/java/org/apache/iceberg/rest/RestCatalog.java

+        .build();
+
+    // TODO - This should come from the server side.
+    String path = properties.getOrDefault("create-namespace-path", "namespace");


what is this about?

This is left over from a previous idea I was toying with.

I will remove this and others like it. It would be pretty hard to deal with it from the iceberg library if users were rewriting the path, given that it contains path variables and things at times. This is just an artifact I forgot to remove. Good catch.

* Use true immutable objects that are type-safe, thread-safe, null-safe * Get builder classes for free * Get JSON serialization/deserialization for free (this will also be helpful for the REST-based Catalog apache#3424) This is relying on https://immutables.github.io/ (Apache License 2.0), which allows generating immutable objects and builders via annotation processing. * Immutable objects are serialization ready (including JSON and its binary forms) * Supports lazy, derived and optional attributes * Immutable objects are constructed once, in a consistent state, and can be safely shared * Will fail if mandatory attributes are missing * Cannot be sneakily modified when passed to other code * Immutable objects are naturally thread-safe and can therefore be safely shared among threads * No excessive copying * No excessive synchronization * Object definitions are pleasant to write and read * No boilerplate setter and getters * No ugly IDE-generated hashCode, equals and toString methods that end up being stored in source control.

kbendick · 2022-01-18T21:42:08Z

I'm closing this as:

Its rather old
We had multiple rounds of review to get some of the spec in.
This no longer reflects really what we agreed on.

I'll be opening a new PR (and ideally splitting it off into sub-PRs for easier digestion) ASAP. Was out sick for a week or so after the initial spec was reviewed.

* Use true immutable objects that are type-safe, thread-safe, null-safe * Get builder classes for free * Get JSON serialization/deserialization for free (this will also be helpful for the REST-based Catalog apache#3424) This is relying on https://immutables.github.io/ (Apache License 2.0), which allows generating immutable objects and builders via annotation processing. * Immutable objects are serialization ready (including JSON and its binary forms) * Supports lazy, derived and optional attributes * Immutable objects are constructed once, in a consistent state, and can be safely shared * Will fail if mandatory attributes are missing * Cannot be sneakily modified when passed to other code * Immutable objects are naturally thread-safe and can therefore be safely shared among threads * No excessive copying * No excessive synchronization * Object definitions are pleasant to write and read * No boilerplate setter and getters * No ugly IDE-generated hashCode, equals and toString methods that end up being stored in source control.

github-actions bot added API build core labels Oct 29, 2021

kbendick force-pushed the rest-interface-list-namespaces branch from 7376d76 to 95f916e Compare October 30, 2021 01:52

rdblue reviewed Oct 31, 2021

View reviewed changes

api/src/main/java/org/apache/iceberg/exceptions/AuthorizationDeniedException.java Outdated Show resolved Hide resolved

rdblue reviewed Oct 31, 2021

View reviewed changes

core/src/main/java/org/apache/iceberg/rest/RestException.java Outdated Show resolved Hide resolved

rdblue reviewed Oct 31, 2021

View reviewed changes

kbendick commented Oct 31, 2021

View reviewed changes

rdblue reviewed Oct 31, 2021

View reviewed changes

kbendick force-pushed the rest-interface-list-namespaces branch from afc2862 to bcffe95 Compare November 15, 2021 06:09

github-actions bot added the spark label Nov 15, 2021

kbendick commented Nov 15, 2021

View reviewed changes

kbendick force-pushed the rest-interface-list-namespaces branch from 4d05922 to 89d3cec Compare November 15, 2021 09:31

kbendick added 10 commits November 16, 2021 11:20

[WIP] REST based catalog api - Initial interface and partial implemen…

c2dccbf

…tation

Remove RestException constructor with no message

21b169c

Remove RestException constructor with no message

3c26658

Rename auth denied to NotAuthorizedException

5ce62d1

Remove unneeded interfaces from RestTableOperations

0da8807

RestTableOperations doesnt need to be Closeable as the HttpClient sho…

52508c3

…uld be closed by the catalog

Clean up CreateNamespaceRequest and then start work on adding in code…

a48bb2b

… from other branches

WIP adds in standardized response class and start work on testing

6648eef

Add in tests that test the CreateNamespaceRequest via the spec and vi…

cf9d563

…a mocks

Outline what specific fields mean what in CreateNamespaceResponse

5b74cb5

kbendick force-pushed the rest-interface-list-namespaces branch from 1e0d30f to 5b74cb5 Compare November 16, 2021 19:20

jackye1995 reviewed Nov 17, 2021

View reviewed changes

nastra self-requested a review November 18, 2021 16:59

nastra mentioned this pull request Dec 8, 2021

API: Use Immutables for TableIdentifier/Namespace #3688

Closed

kbendick closed this Jan 18, 2022

kbendick deleted the rest-interface-list-namespaces branch January 18, 2022 21:42


		}

		private CreateNamespaceRequest(String namespaceName, Map<String, String> properties) {

		private Consumer<CloseableHttpResponse> defaultErrorHandler = ErrorHandlers.databaseErrorHandler();

		private Builder() {

[WIP] REST Based Catalog #3424

[WIP] REST Based Catalog #3424

Conversation

kbendick commented Oct 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick Nov 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick Nov 15, 2021 • edited Loading

Choose a reason for hiding this comment

jackye1995 Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick commented Jan 18, 2022

kbendick Nov 15, 2021 •

edited

Loading

kbendick Nov 5, 2021 •

edited

Loading

kbendick Nov 15, 2021 •

edited

Loading

jackye1995 Nov 17, 2021 •

edited

Loading