You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2024/12/11 19:28:29 UTC

[PR] Isolate OpenAPI-generated classes in separate modules [polaris]

adutra opened a new pull request, #539:
URL: https://github.com/apache/polaris/pull/539

   This PR creates separate modules for the OpenAPI-generated code:
   
   1. `polaris-management-model`
   2. `polaris-management-service`
   3. `polaris-iceberg-service`
   
   This allows better code encapsulation and also allows integrators to reuse the service classes that Polaris builds on, without bringing in the entire Polaris code.
   
   This PR doesn't change any existing functionality and is a no-op for Polaris users.


-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #539:
URL: https://github.com/apache/polaris/pull/539#discussion_r1880794616


##########
api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationRequest.java:
##########
@@ -73,12 +69,14 @@ public int hashCode() {
 
   @Override
   public String toString() {
-    return """
-        class NotificationRequest {
-            notificationType: %s
-            payload: %s
-        }"""
-        .formatted(toIndentedString(notificationType), toIndentedString(payload));
+    return "class NotificationRequest {\n"

Review Comment:
   Using Java 11 compatibility in these modules to maximize reusability.



-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #539:
URL: https://github.com/apache/polaris/pull/539#discussion_r1881100031


##########
api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationRequest.java:
##########
@@ -22,10 +22,6 @@
 import io.swagger.annotations.ApiModelProperty;
 import java.util.Objects;
 
-@jakarta.annotation.Generated(

Review Comment:
   You're right, it shouldn't be there.



-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #539:
URL: https://github.com/apache/polaris/pull/539#discussion_r1880794289


##########
api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationRequest.java:
##########
@@ -22,10 +22,6 @@
 import io.swagger.annotations.ApiModelProperty;
 import java.util.Objects;
 
-@jakarta.annotation.Generated(

Review Comment:
   I believe this annotation is improperly placed, but happy to put it back if I'm wrong.



-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on PR #539:
URL: https://github.com/apache/polaris/pull/539#issuecomment-2538035844

   > Note: this change is also required to support 2 runtimes.
   
   We should discuss the two runtimes during the sync.


-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #539:
URL: https://github.com/apache/polaris/pull/539


-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #539:
URL: https://github.com/apache/polaris/pull/539#discussion_r1880795066


##########
api/iceberg-service/src/main/java/org/apache/polaris/service/types/TableUpdateNotification.java:
##########
@@ -166,19 +168,25 @@ public static final class Builder {
     private Builder() {}
 
     public final Builder tableName(String tableName) {
-      Preconditions.checkArgument(tableName != null, "Null table name supplied");
+      if (tableName == null) {

Review Comment:
   No Guava in these modules, to keep them small and concise.



-- 
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] Isolate OpenAPI-generated classes in separate modules [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #539:
URL: https://github.com/apache/polaris/pull/539#issuecomment-2536967682

   Note: this change is also required to support 2 runtimes.


-- 
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