nit: fix admin tool log level and comments #1626
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous WARNING log levels seems to work, but WARN aligns better with standard Quarkus log levels. Fixes apache#1612
dimas-b
requested review from
adutra,
ashvina,
dennishuo,
eric-maynard,
jackye1995,
jbonofre and
vvcephei
as code owners
dimas-b
requested review from
collado-mike,
snazy,
RussellSpitzer,
takidau,
MonkeyCanCode,
flyrain,
ebyhr,
HonahX,
singhpk234 and
pingtimeout
as code owners
flyrain
approved these changes
May 20, 2025
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
adutra
approved these changes
May 20, 2025
There was a problem hiding this comment.
LGTM
TBH I think WARNING was ok: https://quarkus.io/guides/logging#use-log-levels – see Table 1.
But it's better to use standard level names anyways. 👍
adnanhemani
approved these changes
May 20, 2025
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
snazy
added a commit
to snazy/polaris
that referenced
this pull request
May 22, 2025
* main: Update docker.io/prom/prometheus Docker tag to v3.4.0 (apache#1602) * Site: Update production configuration page (apache#1606) * main: Update dependency com.google.cloud:google-cloud-storage-bom to v2.52.3 (apache#1623) * main: Update dependency boto3 to v1.38.19 (apache#1622) * Remove Bouncy Castle dependency usage from PemUtils (apache#1318) - Added PEM format parsing in PemUtils - Added unit test for PemUtils for empty file and multiple PEM objects - Removed Bouncy Castle Provider dependency from service common module - Removed Bouncy Castle Provider dependency from quarkus service module * Site: Add a page for policy management (apache#1600) * [Policy Store | Management Spec] Add policy privileges to spec and update admin service impl (apache#1529) This PR adds new policy related privileges to polaris-management-api.yml and update PolarisAdminService to allow granting new privileges * Spec: Add SigV4 Auth Support for Catalog Federation (apache#1506) * Spec changes for SigV4 Auth Support for Catalog Federation * Extract service identity info as a nested object * nit: fix admin tool log level and comments (apache#1626) The previous WARNING log levels seems to work, but WARN aligns better with standard Quarkus log levels. Fixes apache#1612 * Doc: switch to use iceberg-aws-bundle jar (apache#1609) * main: Update dependency org.mockito:mockito-core to v5.18.0 (apache#1630) * main: Update dependency boto3 to v1.38.20 (apache#1631) * Require explicit user-consent to enable HadoopFileIO (apache#1532) Using `HadoopFileIO` in Polaris can enable "hidden features" that users are likely not aware of. This change requires users to manually update the configuration to be able to use `HadoopFileIO` in way that highlights the consequences of enabling it. This PR updates Polaris in multiple ways: * The default of `SUPPORTED_CATALOG_STORAGE_TYPES` is changed to not include the `FILE` storage type. * Respect the `ALLOW_SPECIFYING_FILE_IO_IMPL` configuration on namespaces, tables and views to prevent setting an `io-impl` value for anything but one of the configured, supported storage-types. * Unify validation code in a new class `IcebergPropertiesValidation`. * Using `FILE` or `HadoopFileIO` now _also_ requires the explicit configuration `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS=true`. * Added production readiness checks that trigger when `ALLOW_INSECURE_STORAGE_TYPES_ACCEPTING_SECURITY_RISKS` is `true` or `SUPPORTED_CATALOG_STORAGE_TYPES` contains `FILE` (defaults and per-realm). * The two new readiness checks are considered _severe_. Severe readiness-errors prevent the server from starting up - unless the user explicitly configured `polaris.readiness.ignore-security-issues=true`. Log messages and configuration options explicitly use "clear" phrases highlighting the consequences. With these changes it is intentionally extremely difficult to start Polaris with HadoopFileIO. People who work around all these safety nets must have realized that what they are doing. A lot of the test code relies on `FILE`/`HadoopFileIO`, those tests got all the configurations to let those tests continue to work as they are, bypassing the added security safeguards. --------- Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@dremio.com> --------- Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: Yufei Gu <yufei@apache.org> Co-authored-by: David Handermann <exceptionfactory@apache.org> Co-authored-by: Honah (Jonas) J. <honahx@apache.org> Co-authored-by: Rulin Xing <xjdkcsq3@gmail.com> Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@dremio.com> Co-authored-by: MonkeyCanCode <yongzheng0809@gmail.com>
adnanhemani
pushed a commit
to adnanhemani/polaris
that referenced
this pull request
May 22, 2025
The previous WARNING log levels seems to work, but WARN aligns better with standard Quarkus log levels. Fixes apache#1612
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous WARNING log levels seems to work, but WARN aligns better with standard Quarkus log levels.
Fixes #1612