adutra commented on code in PR #3941: URL: https://github.com/apache/polaris/pull/3941#discussion_r2891205044
########## runtime/defaults/src/main/resources/application.properties: ########## @@ -58,7 +58,7 @@ quarkus.http.body.handle-file-uploads=false quarkus.http.limits.max-body-size=10240K quarkus.http.cors.origins=http://localhost:8080 -quarkus.http.cors.methods=PATCH, POST, DELETE, GET, PUT +quarkus.http.cors.methods=PATCH, POST, DELETE, GET, PUT, HEAD, OPTIONS Review Comment: That is the default value in Quarkus, so I wonder if it's simpler to just comment this out? Or better yet: I think we should include `quarkus.http.cors.enabled` (it's probably an oversight), and comment out all the rest: ```properties quarkus.http.cors.enabled=false #quarkus.http.cors.origins=http://example.com:8080 #quarkus.http.cors.methods=PATCH, POST, DELETE, GET, PUT, OPTIONS, HEAD #quarkus.http.cors.headers=* #quarkus.http.cors.exposed-headers=* #quarkus.http.cors.access-control-max-age=PT10M #quarkus.http.cors.access-control-allow-credentials=true ``` WDYT? ########## site/content/in-dev/unreleased/configuration/configuring-polaris.md: ########## @@ -87,25 +87,25 @@ For a comprehensive reference of all Polaris configuration options, see the [Con Some often useful Quarkus configuration properties are listed below. Please refer to the [Quarkus documentation](https://quarkus.io/guides/) for more details. -| Configuration Property | Default Value | Description | -|------------------------------------------------------|---------------------------------|-----------------------------------------------------------------------------| -| `quarkus.log.level` | `INFO` | Define the root log level. | -| `quarkus.log.category."org.apache.polaris".level` | | Define the log level for a specific category. | -| `quarkus.default-locale` | System locale | Force the use of a specific locale, for instance `en_US`. | -| `quarkus.http.port` | `8181` | Define the HTTP port number. | -| `quarkus.http.auth.basic` | `false` | Enable the HTTP basic authentication. | -| `quarkus.http.limits.max-body-size` | `10240K` | Define the HTTP max body size limit. | -| `quarkus.http.cors.enabled` | `false` | Enable the HTTP CORS filter. Must be set to `true` for any other CORS property to take effect. | -| `quarkus.http.cors.origins` | | Define the HTTP CORS origins. | -| `quarkus.http.cors.methods` | `PATCH, POST, DELETE, GET, PUT` | Define the HTTP CORS covered methods. | -| `quarkus.http.cors.headers` | `*` | Define the HTTP CORS covered headers. | -| `quarkus.http.cors.exposed-headers` | `*` | Define the HTTP CORS covered exposed headers. | -| `quarkus.http.cors.access-control-max-age` | `PT10M` | Define the HTTP CORS access control max age. | -| `quarkus.http.cors.access-control-allow-credentials` | `true` | Define the HTTP CORS access control allow credentials flag. | -| `quarkus.management.enabled` | `true` | Enable the management server. | -| `quarkus.management.port` | `8182` | Define the port number of the Polaris management server. | -| `quarkus.management.root-path` | | Define the root path where `/metrics` and `/health` endpoints are based on. | -| `quarkus.otel.sdk.disabled` | `true` | Enable the OpenTelemetry layer. | +| Configuration Property | Default Value | Description | +|------------------------------------------------------|------------------------------------------------|------------------------------------------------------------------------------------------------| +| `quarkus.log.level` | `INFO` | Define the root log level. | +| `quarkus.log.category."org.apache.polaris".level` | | Define the log level for a specific category. | +| `quarkus.default-locale` | System locale | Force the use of a specific locale, for instance `en_US`. | +| `quarkus.http.port` | `8181` | Define the HTTP port number. | +| `quarkus.http.auth.basic` | `false` | Enable the HTTP basic authentication. | +| `quarkus.http.limits.max-body-size` | `10240K` | Define the HTTP max body size limit. | +| `quarkus.http.cors.enabled` | `false` | Enable the HTTP CORS filter. Must be set to `true` for any other CORS property to take effect. | +| `quarkus.http.cors.origins` | | Define the HTTP CORS origins. | +| `quarkus.http.cors.methods` | `PATCH, POST, DELETE, GET, PUT, HEAD, OPTIONS` | Define the HTTP CORS covered methods. | Review Comment: What "Default Value" means here is ambiguous. I would rename the column to "Default Value in Polaris" to clarify things. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
