Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix/workflow failure #1250

Merged
merged 6 commits into from
Jan 27, 2025
Merged

Fix/workflow failure #1250

merged 6 commits into from
Jan 27, 2025

Conversation

AssahBismarkabah
Copy link
Collaborator

@AssahBismarkabah AssahBismarkabah commented Jan 11, 2025

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fix ci failure and closes #1246

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

AssahBismarkabah and others added 4 commits January 7, 2025 14:51
Workaround for CI fails because ubuntu-latest version is 24.04 which doesn't support python 3.7.

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
@AssahBismarkabah AssahBismarkabah marked this pull request as draft January 11, 2025 10:11
@AssahBismarkabah AssahBismarkabah force-pushed the fix/workflow-failure branch 4 times, most recently from 9ffa12c to 4d2c212 Compare January 14, 2025 13:04
@bohmber
Copy link
Contributor

bohmber commented Jan 25, 2025

@AssahBismarkabah tested your ci build fix on my fork:
bohmber#1

The ci build is green. There was one failure on the keycloak 23 build. After a re-run it was successful.
The only difference was enable access log for quarkus in AbstractImportIT.java

.withEnv("QUARKUS_HTTP_ACCESS_LOG_ENABLED", "true")

Can you re-run the failing ci jobs on you pr?

@AssahBismarkabah
Copy link
Collaborator Author

AssahBismarkabah commented Jan 25, 2025

@AssahBismarkabah tested your ci build fix on my fork: bohmber#1

The ci build is green. There was one failure on the keycloak 23 build. After a re-run it was successful. The only difference was enable access log for quarkus in AbstractImportIT.java

.withEnv("QUARKUS_HTTP_ACCESS_LOG_ENABLED", "true")

Can you re-run the failing ci jobs on you pr?

Hi @bohmber , It seems to be successful. Can you review the PR so it can be merged and rebased with the other pending PRs? Thanks for your assistance :)

@AssahBismarkabah AssahBismarkabah marked this pull request as ready for review January 25, 2025 11:41
@bohmber
Copy link
Contributor

bohmber commented Jan 25, 2025

@AssahBismarkabah What is the differences between this PR and this one #1246 ?

I don't have enough karma that my review is enough.

@AssahBismarkabah
Copy link
Collaborator Author

AssahBismarkabah commented Jan 25, 2025

@bohmber i guess #1246 can be closed if this is merged, as it attempted to fix the failure in #1246

I didn't even include the quarkus log .withEnv("QUARKUS_HTTP_ACCESS_LOG_ENABLED", "true") in the AbstractImportIT.java so what really changed then ?

@bohmber
Copy link
Contributor

bohmber commented Jan 25, 2025

@AssahBismarkabah I don't know what has changed in the meantime. It could be something github internal.

Copy link
Collaborator

@Motouom Motouom left a comment

Choose a reason for hiding this comment

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

Looks good to me @AssahBismarkabah. But i am unable to find the root cause for the previous failure and the custom attribute is populated locally.

Copy link
Contributor

@bohmber bohmber left a comment

Choose a reason for hiding this comment

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

LGTM

@AssahBismarkabah AssahBismarkabah merged commit ae5dbe7 into main Jan 27, 2025
16 checks passed
@AssahBismarkabah AssahBismarkabah deleted the fix/workflow-failure branch January 27, 2025 11:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants