Skip to content

Ignore Health and Metrics artifacts - #22438

Merged
vmuzikar merged 1 commit into
keycloak:mainfrom
mabartos:ignoreHealth
Dec 8, 2023
Merged

Ignore Health and Metrics artifacts#22438
vmuzikar merged 1 commit into
keycloak:mainfrom
mabartos:ignoreHealth

Conversation

@mabartos

@mabartos mabartos commented Aug 14, 2023

Copy link
Copy Markdown
Member

Closes #22437

@mabartos

Copy link
Copy Markdown
Member Author

@vmuzikar @ahus1 Could you please check it? Thanks

@ahus1 ahus1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the reasons when we did this for database drivers which we excluded from the distribution and when we wanted to avoid the warnings in the log.

I am not sure about the reasons here and what benefits we're expecting from this change. Is is a shorter startup time, and less memory use? This comes with the trade-off of maintainability, as we would need to know about the transitional dependencies of those libraries.

If the benefits are not significant, I'd recommend to keep the code simpler to maintain by not implementing this change.

If the CND team still wants to merge it, I won't veto it, so that's why I just comment here.

@vmuzikar
vmuzikar requested review from Pepo48 and shawkins September 14, 2023 12:28
@mabartos
mabartos marked this pull request as draft November 22, 2023 13:18
@vmuzikar

Copy link
Copy Markdown
Contributor

@mabartos Could you please rebase?

@mabartos

mabartos commented Nov 22, 2023

Copy link
Copy Markdown
Member Author

@vmuzikar I marked the PR as a draft as I've encountered an issue with the micrometer. Even when the feature is disabled, there's some requirement on the Quarkus side to have particular micrometer components available in the core, which is strange. I'll analyze it once it fits my TODO plan with regard to priorities.

EDIT: No problems with it, caused by merging this PR: #24812

@vmuzikar

Copy link
Copy Markdown
Contributor

Unless there'd be a significant perf benefit in excluding these artifacts, I would not prioritize it at the moment. My 2 cents. :)

Comment on lines 100 to 102

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation in the: #25012 (comment)

@vmuzikar

vmuzikar commented Dec 4, 2023

Copy link
Copy Markdown
Contributor

@mabartos Could you please rebase?

@vmuzikar vmuzikar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @mabartos.

Closes keycloak#22437

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
@cypress

cypress Bot commented Dec 9, 2023

Copy link
Copy Markdown

3 flaky tests on run #10181 ↗︎

0 1090 108 0 Flakiness 3

Details:

Ignore Health and Metrics artifacts (#22438)
Project: Keycloak Admin UI Commit: 0a95159e07
Status: Passed Duration: 07:22 💡
Started: Dec 11, 2023 12:22 AM Ended: Dec 11, 2023 12:30 AM
Flakiness  masthead_test.spec.ts • 1 flaky test • chrome

View Output

Test Artifacts
Masthead tests > Desktop view > Go to account console and back to admin console Test Replay Screenshots
Flakiness  clients_test.spec.ts • 1 flaky test • firefox

View Output

Test Artifacts
Clients test > Client details - Client scopes subtab > Should search non-existent client scope by name Screenshots
Flakiness  client_registration_policies.spec.ts • 1 flaky test • firefox

View Output

Test Artifacts
Client registration policies tab > Authenticated client policies subtab > add authenticated client registration policy Screenshots

Review all test suite changes for PR #22438 ↗︎

ShefeeqPM pushed a commit to ShefeeqPM/keycloak that referenced this pull request Jan 27, 2024
Closes keycloak#22437

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Signed-off-by: ShefeeqPM <86718986+ShefeeqPM@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ignore Health and Metrics artifacts

3 participants