snazy commented on code in PR #2012:
URL: https://github.com/apache/polaris/pull/2012#discussion_r2204521913
##########
spec/polaris-management-service.yml:
##########
@@ -1056,6 +1056,12 @@ components:
type: string
description: endpoint for STS requests (optional). If not set,
defaults to 'endpoint'.
example: "https://sts.example.com:1234";
+ pathStyleAccess:
+ type: boolean
+ description: >-
+ Whether S3 requests to files in this catalog should use
'path-style addressing for buckets'.
+ Default: false.
Review Comment:
TBH, I do not understand the whole discussion about "is this a breaking
change".
As Polaris is primarily an Iceberg REST catalog, we _have to_ let JSON
parsing _ignore_ unknown properties, because that is a requirement from Iceberg
REST. There is also exactly _one_ JSON parser (`ObjectMapper`) for all
endpoints. The situation is unfortunate but a consequence of generating the
OpenAPI code. Jackson itself allows much more fine grained control over how
types are (de)serialized and allows back/forwards compatibility targeting
functionality like `@JsonView` - but that's not usable w/ the code generation
approach.
So fact is that unknown properties in JSON objects in any Polaris REST
payload have to be ignored. As that fact leaks to all Polaris APIs, it also
leaks to clients. This means, that a new property is not a breaking change. If
clients fail on unknown properties, it's a bug in those client.
--
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-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
