Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Avoid executing OpenAPI build filters twice on build #38070

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Avoid executing OpenAPI build filters twice on build #38070

merged 1 commit into from
Feb 6, 2024

Conversation

mcruzdev
Copy link
Contributor

@mcruzdev mcruzdev commented Jan 5, 2024

Fixes #37901

@mcruzdev
Copy link
Contributor Author

mcruzdev commented Jan 5, 2024

Hi @phillip-kruger, can you take a look in my solution? I am implementing tests.

@mcruzdev mcruzdev marked this pull request as ready for review January 5, 2024 22:37
@mcruzdev mcruzdev force-pushed the issue-37901 branch 2 times, most recently from 8f28453 to a8ecb2f Compare January 8, 2024 17:32
@gsmet gsmet requested a review from phillip-kruger January 9, 2024 09:21
@mcruzdev mcruzdev force-pushed the issue-37901 branch 4 times, most recently from a4cb12b to a89de30 Compare January 15, 2024 12:46
@mcruzdev mcruzdev requested a review from gastaldi January 15, 2024 21:48
@gastaldi
Copy link
Contributor

Can you please squash all the commits before we merge this?

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@mcruzdev mcruzdev force-pushed the issue-37901 branch 2 times, most recently from 8cc2ad0 to c1efe46 Compare January 16, 2024 03:30
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@mcruzdev mcruzdev force-pushed the issue-37901 branch 3 times, most recently from b8e47c9 to b6071a0 Compare January 22, 2024 14:01
@mcruzdev
Copy link
Contributor Author

Hi @gastaldi, how are you? I did the squash!

@gastaldi
Copy link
Contributor

I'd like @phillip-kruger's review before proceeding 👍🏻

@gastaldi gastaldi changed the title Evict twice callings on build Avoid twice callings on build Jan 22, 2024
@gastaldi
Copy link
Contributor

BTW I renamed the PR (s/Evict/Avoid) which is what I guess you mean here. Can you also change the commit message?

@gastaldi gastaldi changed the title Avoid twice callings on build Avoid executing OpenAPI build filters twice on build Jan 22, 2024
@mcruzdev
Copy link
Contributor Author

BTW I renamed the PR (s/Evict/Avoid) which is what I guess you mean here. Can you also change the commit message?

Thank you! I will change here

@mcruzdev mcruzdev force-pushed the issue-37901 branch 2 times, most recently from 325ff3f to 9d4a6c3 Compare January 23, 2024 14:01
@mcruzdev mcruzdev mentioned this pull request Jan 31, 2024
Closed
@phillip-kruger
Copy link
Member

@mcruzdev Please can you resolve conflicts, I'll have a look once that is done. Thanks :)

@mcruzdev mcruzdev force-pushed the issue-37901 branch 2 times, most recently from 729c5c6 to eb21de0 Compare February 5, 2024 14:39
@mcruzdev
Copy link
Contributor Author

mcruzdev commented Feb 5, 2024

@mcruzdev Please can you resolve conflicts, I'll have a look once that is done. Thanks :)

Done @phillip-kruger, thank you!

@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 5, 2024
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@mcruzdev
Copy link
Contributor Author

mcruzdev commented Feb 6, 2024

I will fix this one tomorrow!

@mcruzdev mcruzdev force-pushed the issue-37901 branch 2 times, most recently from eeb7331 to e78a7dc Compare February 6, 2024 17:52
@mcruzdev
Copy link
Contributor Author

mcruzdev commented Feb 6, 2024

Hi @phillip-kruger, I fixed the issue, I think that will work well.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 6, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit dc0e6dd.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-health/deployment

✖ io.quarkus.smallrye.health.test.HealthCheckContextPropagationTest.testContextPropagatedToHealthChecks - History

  • `3 expectations failed.
    JSON path status doesn't match.
    Expected: is "UP"
    Actual: DOWN

JSON path checks.status doesn't match.
Expected: iterable containing ["UP"]
Actual: <[DOWN]>

JSON path checks.name doesn't match.
Expected: iterable containing ["io.quarkus.smallrye.health.test.HealthCheckContextPropagationTest$ContextualHC"]
Actual: <[io.quarkus.smallrye.health.test.HealthCheckContextPropagationTest_ContextualHC_ClientProxy]>
` - java.lang.AssertionError

java.lang.AssertionError: 
3 expectations failed.
JSON path status doesn't match.
Expected: is "UP"
  Actual: DOWN

JSON path checks.status doesn't match.
Expected: iterable containing ["UP"]

@phillip-kruger phillip-kruger merged commit e037514 into quarkusio:main Feb 6, 2024
24 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Feb 6, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillip-kruger phillip-kruger phillip-kruger approved these changes

@gastaldi gastaldi Awaiting requested review from gastaldi

Assignees
No one assigned
Labels
area/openapi area/smallrye triage/flaky-test
Projects
None yet
Milestone
3.9.0.CR1
Development

Successfully merging this pull request may close these issues.

OpenAPI filters run twice when storing
3 participants
@mcruzdev @gastaldi @phillip-kruger