-
Notifications
You must be signed in to change notification settings - Fork 334
feat: Improve PolarisAdminTool default output #2961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Improve PolarisAdminTool default output #2961
Conversation
singhpk234
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM ! thanks @MonkeyCanCode
runtime/admin/src/main/java/org/apache/polaris/admintool/PolarisAdminTool.java
Outdated
Show resolved
Hide resolved
runtime/admin/src/main/java/org/apache/polaris/admintool/PolarisAdminTool.java
Outdated
Show resolved
Hide resolved
dimas-b
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to the general direction of this improvement. Thanks for working on this @MonkeyCanCode ! Also +1 to @snazy 's comments :)
snazy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
+1
* Pass AccessConfig into FileIOFactory (apache#2937) it should not be the responsibility of the `FileIOFactory` to know how to infer the `AccessConfig` * Remove EclipseLink Persistence Backend (apache#2963) as per ML decision the deprecated eclipselink backend is being removed in the next version: https://lists.apache.org/thread/16bj5kngf2kfhqv3noxwfm7h9wlzvhyv * feat: Improve PolarisAdminTool default output (apache#2961) * feat: Improve PolarisAdminTool default output * feat: Improve PolarisAdminTool default output * Add Polaris Community Meeting 2025-10-30 (apache#2973) * Remove legacy management endpoints (apache#2276) * Build/nit: cache output of `generatedMarkdownDocs` (apache#2967) `(Java)Exec` tasks are not cacheable by default, as annotated with `@DisableCachingByDefault`. Adding an `outputs.cacheIf { true }` enables caching on those tasks. * Build: Helper to get effective ASF project metadata (apache#2969) This change "bundles" the information of `AsfProject` and the `PublishingHelperExtensions`, which is what the code in `configurePom.kt` did. Bundling these objects allows other consumers, like CycloneDX SBOM generation, to access that same information without having to query remote systems (whimsey.apache.org) again. * Alternative, concise PR template (apache#2945) This PR proposes an alternative PR template that is much shorter, and removes all the redundant claims. It also links to the contribution guidelines for further guidance. * Add docs how to add `Server` header to HTTP responses (apache#2941) * Prefer PolarisPrincipal over SecurityContext (apache#2932) The general idea is that `SecurityContext` comes from `jakarta.ws.rs` and there is no reason for non-REST related classes to rely on those details. Instead, once preprocessing of a REST-request has inferred the `PolarisPrincipal` all inner/core code should rely on only that. Note that this simplifies a bunch of tests that had to create their own `SecurityContext` around the principal that they wanted to use, thus having to decide how to implement `isUserInRole` and the other methods. * Last merged commit f934443 --------- Co-authored-by: Christopher Lambert <xn137@gmx.de> Co-authored-by: Yong Zheng <yongzheng0809@gmail.com> Co-authored-by: JB Onofré <jbonofre@apache.org> Co-authored-by: Alexandre Dutra <adutra@apache.org> Co-authored-by: fivetran-arunsuri <103934371+fivetran-arunsuri@users.noreply.github.com>
What changes were proposed in this pull request?
Polaris admin tool was default to show a brief msg which then ask user to use argument
helpto get more info (which is a kinds no-ops and force user to rerun the command with additional arguments in order to learn more) such as following:With the change from this PR, I am purposing we should default to
helpalong with original tool message line such as following:Why are the changes needed?
Does this PR introduce any user-facing change?
IMO, this is provide better user experience and it doesn't make much sense to ask user to type same command and an extra
helpargument to learn more about the tool.How was this patch tested?
N/A
CHANGELOG.md
N/A