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

gRPC - archive with a MutinyBean implementor is a bean archive #19902

Merged
merged 2 commits into from
Sep 6, 2021
Merged

gRPC - archive with a MutinyBean implementor is a bean archive #19902

merged 2 commits into from
Sep 6, 2021

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Sep 3, 2021

No description provided.

@mkouba
ArC - make it possible to identify additional bean archives
c91431c
@mkouba
gRPC - register additional bean archives
13bc4d2 
- an application archive that contains a MutinyBean implementor is an
additional bean archive
- resolves quarkusio#19864
@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/grpc gRPC labels Sep 3, 2021
mkouba
mkouba commented Sep 3, 2021
View reviewed changes
...ns/arc/deployment/src/main/java/io/quarkus/arc/deployment/BeanArchivePredicateBuildItem.java
* By default, only explict/implicit bean archives (as defined by the spec) are considered during the bean discovery. However,
* extensions can register a logic to identify additional bean archives.
*/
public final class BeanArchivePredicateBuildItem extends MultiBuildItem {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the name of this build item. The other alternative was AdditionalBeanArchiveBuildItem but given the fact that the item does not represent a bean archive but a logic that identifies a bean archive I decided for BeanArchivePredicateBuildItem.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think BeanArchivePredicateBuildItem makes sense.

@mkouba mkouba changed the title Issue 19864 gRPC - archive with a MutinyBean implementor is a bean archive Sep 3, 2021
Ladicek
Ladicek approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

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

Test for the original issue would be nice, but I see how that's not straightforward to add.

@mkouba
Copy link
Contributor Author

mkouba commented Sep 3, 2021

Test for the original issue would be nice, but I see how that's not straightforward to add.

I don't know of a way how to test it TBH...

michalszynkiewicz
michalszynkiewicz reviewed Sep 3, 2021
View reviewed changes
extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java
@Override
public boolean test(ApplicationArchive archive) {
// Every archive that contains a generated implementor of MutinyBean is considered a bean archive
return !archive.getIndex().getKnownDirectImplementors(GrpcDotNames.MUTINY_BEAN).isEmpty();
Copy link
Member

Choose a reason for hiding this comment

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

is archive.getIndex() always != null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so. But it's a good point that we could be more defensive here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, we don't test the nullability in the BeanArchiveProcessor either. So it's probably OK.

@michalszynkiewicz
Copy link
Member

nice! Thanks Martin!

@mkouba mkouba added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport? labels Sep 3, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 3, 2021 •
edited
Loading

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 13bc4d2

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✖ JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/amazon-lambda/deployment 
! Skipped: docs extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 6 more

📦 extensions/amazon-lambda/deployment

✖ io.quarkus.amazon.lambda.deployment.testing.LambdaDevServicesContinuousTestingTestCase.testLambda line 41 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 5, 2021

Failing Jobs - Building 13bc4d2

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✖ JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/vertx-http/deployment 
! Skipped: core/test-extension/deployment docs extensions/agroal/deployment and 287 more

📦 extensions/vertx-http/deployment

✖ io.quarkus.vertx.http.testrunner.tags.IncludeTagsTestCase.checkTestsAreRun line 52 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Failed to wait for test run 2 State{lastRun=1, running=true, inProgress=false, run=2, passed=2, failed=0, skipped=0, isBrokenOnly=false, isTestOutput=false, isInstrumentationBasedReload=false, isLiveReload=true}
	at io.quarkus.test.ContinuousTestingTestUtils.waitForNextCompletion(ContinuousTestingTestUtils.java:43)
	at io.quarkus.vertx.http.testrunner.tags.IncludeTagsTestCase.checkTestsAreRun(IncludeTagsTestCase.java:52)

@mkouba mkouba merged commit ed1fc5f into quarkusio:main Sep 6, 2021
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Sep 6, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 6, 2021
@mkouba
Copy link
Contributor Author

mkouba commented Sep 6, 2021

Windows CI failures do not seem to be related.

@gsmet gsmet modified the milestones: 2.3 - main, 2.2.2.Final Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manovotn manovotn manovotn approved these changes

@michalszynkiewicz michalszynkiewicz michalszynkiewicz approved these changes

@Ladicek Ladicek Ladicek approved these changes

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) area/grpc gRPC
Projects
None yet
Milestone
2.2.2.Final
Development

Successfully merging this pull request may close these issues.
5 participants
@mkouba @michalszynkiewicz @Ladicek @manovotn @gsmet