-
Notifications
You must be signed in to change notification settings - Fork 357
Remove legacy management endpoints #2276
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
Conversation
|
@flyrain FYI |
|
Hi @adutra , thanks for tagging me. Will take a look soon. |
|
Putting this PR on hold for now to give users more time to adjust to the new endpoints. |
|
I think it's time to revive this PR as there was no feedback in the last two weeks, let's get it in. |
|
@flyrain @singhpk234 would it be OK to include this in 1.2.0? |
|
Should we make it "ready for review"? 😅 |
|
+1 to including into 1.2.0 |
|
Can we remove it at 2.0? I'm a bit paranoid of removing endpoints between minor versions. |
|
Endpoints on the management (Quarkus) interface are not formally considered public API, I guess (they are not callable by clients, only by infra components within the "control plane"), so it should be ok to drop old ones in a minor release, I think. Admin users should be able to transition their monitoring solutions to the new endpoints in 1.1.0 (where both sets are supported) so upgrading to a release where the old endpoints are dropped would be transparent. This is a different use case than supporting older clients, which may run multiple old versions at the same time. However, it's a bit of a grey area 🤔 |
|
I would also request if possible lets move the removal to 2.0, mostly because I am not fully aware of how |
It's exactly the same endpoint. It's just a mirror of |
|
@adutra as 1.2.0 isn't yet "frozen", WDYT of deprecating these in 1.2.0 and removing in 1.3.0? |
WFM, but how do we deprecate an endpoint? Log a warning when it's accessed the first time? |
a8cda01 to
453508b
Compare
A CHANGELOG notice is fine from my POV. |
There you go: #2749 |
453508b to
9523396
Compare
|
Now that 1.2.0 is out, I'm moving this PR to ready state. |
|
Now that 1.2.0 is (nearly) out, I think it's fine to merge this PR for 1.3. |
|
Let's merge right after 1.2.0 is released :) |
|
Are we OK to merge this now? |
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.
+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 <[email protected]> Co-authored-by: Yong Zheng <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: fivetran-arunsuri <[email protected]>
No description provided.