Extension for running Narayana LRA participants #17903

mmusgrov · 2021-06-14T21:36:37Z

This PR adds support for an implementation of the eclipse microprofile-lra project version 1.0 (https://github.com/eclipse/microprofile-lra/releases/tag/1.0). I want to target the extension to core because compensating transactions are an essential component for building reliable services (and in this respect is similar/complementary to the narayana-jta extension which itself is useful for doing each of the compensatory actions that comprise the LRA).

It replaces PR #6763 which was closed as out of date. I was unable to re-open it but the discussion provides useful background and context for this PR.

quarkus-bot · 2021-06-14T21:51:59Z

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

Failing Jobs - Building `8297860`

Status	Name	Step	Test failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Build`	⚠️ Check →	Logs	Raw logs

quarkus-bot · 2021-06-15T09:00:42Z

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

Failing Jobs - Building `9761d8b`

Status	Name	Step	Test failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Verify extension dependencies`	⚠️ Check →	Logs	Raw logs

quarkus-bot · 2021-06-15T12:13:10Z

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

Failing Jobs - Building `7a506d9`

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

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/narayana-lra

✖ org.acme.quickstart.lra.LRAParticipantTest.testLRA - More details - Source on GitHub

...na-lra/deployment/src/main/java/io/quarkus/narayana/lra/deployment/NarayanaLRAProcessor.java

NahuelOlgiati · 2021-06-16T19:56:48Z

docs/src/main/asciidoc/lra-guide.adoc

+
+[source,bash]
+----
+quarkus.lra.coordinator-url=http://localhost:8080/lra-coordinator


Maybe "quarkus.narayana.lra.coordinator-url" is better than "quarkus.lra.coordinator-url"

Makes sense, I will add this change too.

But I will wait a while for anyone else who wants to review the PR before pushing a commit (since the PR check took about 5 hours last time). I will give it a few days and then ping someone from the quarkus team to check whether or not your review is sufficient to get the PR merged.

Actually, I don't think we include implementation names for other properties that are tied to the MP specs. If we would change the impl in the future (not that I expect that but never say never) this can be an issue. I would prefer quarkus.lra.coordinator.url (note . instead of -) as there might be more properties under coordinator in future. The quarkus. prefix is enough to display this is coming from quarkus.

So it's a land grab, I have no issues with that!

So on a second reading I am going to keep the property name as it was (namely, quarkus.lra.coordinator-url). - it's how the quarkus config property naming system is meant to work.

@NahuelOlgiati (Nahuel Olgiati) this was your only review comment that I declined. If that's okay and if the rest of the updates I made to the PR are okay too then if you have the permissions, will you mark it as approved.

mmusgrov · 2021-06-17T17:00:53Z

@quarkusio/committerhelp I have had one review so far, what is the process for getting things merged once I have addressed all reviewer comments. Note that I am still waiting for the original reviewer (@xstefank ) to provide his review.

xstefank · 2021-06-17T08:44:10Z

docs/src/main/asciidoc/lra-guide.adoc

+
+== Introduction
+
+The LRA (short for Long Running Action) participant extension is useful in microservice based


Suggested change

The LRA (short for Long Running Action) participant extension is useful in microservice based

The LRA (short for Long Running Actions) participant extension is useful in microservice based

I think this should be aligned with the spec. But I don't insist.

An LRA is a long running action.

ok, I think the specs is plural https://github.com/eclipse/microprofile-lra#long-running-actions-for-microprofile but I have no issue with this.

A participant participates in a Long Running Action.

core/deployment/src/main/java/io/quarkus/deployment/Capability.java

xstefank · 2021-06-18T07:41:45Z

docs/src/main/asciidoc/lra-guide.adoc

+
+[source,bash]
+----
+quarkus.lra.coordinator-url=http://localhost:8080/lra-coordinator


Actually, I don't think we include implementation names for other properties that are tied to the MP specs. If we would change the impl in the future (not that I expect that but never say never) this can be an issue. I would prefer quarkus.lra.coordinator.url (note . instead of -) as there might be more properties under coordinator in future. The quarkus. prefix is enough to display this is coming from quarkus.

xstefank · 2021-06-18T07:44:25Z

docs/src/main/asciidoc/lra-guide.adoc

+quarkus.lra.coordinator-url=http://localhost:8080/lra-coordinator
+----
+
+For a narayana coordinator the path component of the url is normally `lra-coordinator`.


not sure if the Narayana shouldn't be capitalized.

On a second reading, capitalising it is a bit shouty, I don't see any problem with lower case.

xstefank · 2021-06-18T07:48:32Z

docs/src/main/asciidoc/lra-guide.adoc

+the coordinator or for repairing the network, respectively). To fulfil this task the
+coordinator must have access to durable storage for its logs (via a filesystem or in
+a database).  At the time of writing, managing coordinators is the responsibility of
+the user. An "out-of-the-box" solution will be forthcoming.


Why isn't the docker image for the coordinator also out-of-the-box? It's an external service like jaeger, kafka, prometheus, etc. I would remove this last sentence.

Because the user has to manage it.

Example "out-of-the-box" solutions will be WildFly and an OCP operator.

xstefank · 2021-06-18T08:40:26Z

...ts/narayana-lra/src/main/java/org/acme/quickstart/lra/coordinator/TransactionalResource.java

+    @PUT
+    @Path("compensate")
+    @Compensate
+    public Response compensateWork(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) URI lraId, String userData) {


userData is unused

That's a shame, users would find it extremely useful. I will check whether or not it still works in the narayana implementation of the LRA spec and if not then I will remove it.

we can have this on top of the spec in the Narayana implementation as an experimental feature, agreed it would be nice to have.

It's not an experimental feature, it is part of the narayana implementation of LRAs and has been present, I believe (I would need to look through the history) ever since I took the spec to the MP group. Don't forget that the MP-LRA spec process removed a lot of useful stuff from the implementation and that the narayana LRA implementation is not restricted to what the spec says, it is a transactional solution that just happens to implement all of the requirements of the MP-LRA spec.

true, not experimental, additional, not speced. Sorry.

xstefank · 2021-06-18T08:40:46Z

...ts/narayana-lra/src/main/java/org/acme/quickstart/lra/coordinator/TransactionalResource.java

+    @PUT
+    @Path("complete")
+    @Complete
+    public Response completeWork(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) URI lraId, String userData) {


same, userData can be removed

Same response.

xstefank · 2021-06-18T08:41:31Z

integration-tests/narayana-lra/src/main/resources/application.properties

+quarkus.http.test-port=8081
+quarkus.http.host=${HOST:localhost}
+#quarkus.http.host=0.0.0.0
+#%test.quarkus.http.host=0.0.0.0


comments can also be removed now I think

Well they should work so I'd left it in so that we don't forget. I still need to talk to the resteasy integration team about the problem. But if you think it's going to be an issue I can remove it.

If they are not used I would suggest removing them, we can add them when they add some value but now it only means that anyone reading the code will stop thinking why this is in here and why it is commented.

You missed my point but I can remove them.

xstefank · 2021-06-18T09:21:56Z

.../narayana-lra/src/test/java/org/acme/quickstart/lra/LRAParticipantTestResourceLifecycle.java

+
+    @Override
+    public Map<String, String> start() {
+        registry = new GenericContainer<>("jbosstm/lra-coordinator:5.12.0.Final")


Maybe we can use latest here or parse the version dynamically from the properties? More a question to the quarkus team in how this is handled elsewhere.

It's a test. And other integration tests use a similar approach.

ok, just to remember to bump this version when the Narayana update will be done all the time seems a little overkill. Not sure if dependabot can handle such a scenario.

When we migrate to a new version of narayana the version will need updating in various places, I'm still not seeing the benefit of making this particular change generic (if I could even figure out a clean way to do that).

dependabot won't update it.
But, I believe it's fine.

Just add a periodic reminder to update the version.

What is a periodic reminder?

a periodic todo task that remind you to update the version once in a while.

I am not sure what you are asking us to do but we try to track our users and update accordingly, for example we already do this with wildfly and quarkus is another platform we monitor,

xstefank · 2021-06-18T09:22:36Z

.../narayana-lra/src/test/java/org/acme/quickstart/lra/LRAParticipantTestResourceLifecycle.java

+    public Map<String, String> start() {
+        registry = new GenericContainer<>("jbosstm/lra-coordinator:5.12.0.Final")
+                //.withNetworkMode("host"); // fails with: "Container doesn't expose any ports"
+                //.withExposedPorts(90010); // and exposing one then fails with: "Requested port (90010) is not mapped"


Not sure if this has value, do you think that others will run into similar issues?

It was an attempt at working around a resteasy integration issue about which I still need to talk to the resteasy integration team . But, again, if you think it's going to be an issue I can remove it. On the hand leaving it in will help other developers who hit the same issue.

maybe you can open a new issue if this needs to be fixed elsewhere where you will reference a branch in your fork with the left comments to be fixed and then you can open a follow-up PR updating to the correct configuration. So please remove the unused code.

I don't think so. But if you insist then I will remove them.

The only exposed port is 8080/tcp. So you would need .withExposedPorts(8080) and retrieved the mapped port.

I already to that, if you check the code in the PR you can see that we are already exposing 8080.

xstefank

Thanks, LGTM but someone from the Quarkus team should check if the extension stuff is in the right places.

quarkus-bot · 2021-06-22T20:01:14Z

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

Failing Jobs - Building `082a342`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Test failures	Logs	Raw logs
✖	Native Tests - Data3	`Build`	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 devtools/cli

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults line 47 - More details - Source on GitHub

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides line 77 - More details - Source on GitHub

⚙️ JVM Tests - JDK 16 #

📦 devtools/cli

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults line 47 - More details - Source on GitHub

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides line 77 - More details - Source on GitHub

📦 integration-tests/grpc-hibernate-reactive

✖ com.example.reactive.ReactiveServiceTest.shouldWatchAndAddMultipleTimes line 45 - More details - Source on GitHub

mmusgrov · 2021-06-24T08:44:45Z

If anyone from the quarkusio team is looking at this, the test failures concern io.quarkus.cli and look unrelated to the PR. Note also that the initial CI run passed and the second run included minor changes only after requests by the reviewers.

cescoffier

Lots of comments are actually questions to satisfy my own curiosity.

core/deployment/src/main/java/io/quarkus/deployment/Capability.java

docs/src/main/asciidoc/lra-guide.adoc

cescoffier · 2021-06-29T07:41:26Z

.../narayana-lra/runtime/src/main/java/io/quarkus/narayana/lra/runtime/NarayanaLRARecorder.java

+@Recorder
+public class NarayanaLRARecorder {
+    public void setConfig(final LRAConfiguration config) {
+        if (System.getProperty(NarayanaLRAClient.LRA_COORDINATOR_URL_KEY) == null) {


So, it can be set from a sys variable, but not an env one? In addition, it overrides the configured value.
Is it because the TCK expect it, or what's the reason behind it?

The configured value can be set using -Dquarkus.lra.coordinator-url and from the QUARKUS_LRA_COORDINATOR_URL env variable.

cescoffier · 2021-06-29T07:42:28Z

integration-tests/narayana-lra/pom.xml

+        </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-resteasy-deployment</artifactId>


I would test with classic resteasy but also with resteasy reactive. Should not change anything, but we never know.

I will give it another go. I did try modifying the dependencies and got errors like:

Caused by: java.lang.IllegalStateException: Please make sure there is only one provider of the following capabilities: capability io.quarkus.rest is provided by: - io.quarkus:quarkus-resteasy-reactive::jar:999-SNAPSHOT - io.quarkus:quarkus-resteasy::jar:999-SNAPSHOT

I couldn't figure who was still pulling the non reactive version but I will investigate further.

See my earlier comment above (#17903 (comment)) about how we only use the async API anyway.

You need to remove io.quarkus:quarkus-resteasy, both can't be used at the same time.

That is what I did.

Hum, that's kind of annoying. Does your implementation pull the classic Resteasy. We would need to have a look.

integration-tests/narayana-lra/src/main/resources/application.properties

cescoffier · 2021-06-29T07:43:59Z

integration-tests/narayana-lra/src/test/java/org/acme/quickstart/lra/LRAParticipantTest.java

+    }
+
+    // send an HTTP GET request to an endpoint
+    private static String coordinatorGetRequest(String endpoint, int[] expectedResponseCodes, boolean readResponse)


Why not using restassured for this? You can pass absolute URLs too.

The routine uses Jupiter:

assertTrue(Arrays.stream(expectedResponseCodes).anyMatch(i -> i == responseCode), "unexpected response code: GET " + endpoint);

cescoffier · 2021-06-29T07:45:02Z

.../narayana-lra/src/test/java/org/acme/quickstart/lra/LRAParticipantTestResourceLifecycle.java

+
+    @Override
+    public Map<String, String> start() {
+        registry = new GenericContainer<>("jbosstm/lra-coordinator:5.12.0.Final")


dependabot won't update it.
But, I believe it's fine.

mmusgrov · 2021-07-01T09:47:12Z

@cescoffier I responded to your comments but I won't push changes until I know you are happy with how I plan to address those comments that require updates to the extension. So if you wouldn't mind, can you confirm whether or not my responses are adequate?

cescoffier · 2021-07-05T05:56:25Z

.../narayana-lra/src/test/java/org/acme/quickstart/lra/LRAParticipantTestResourceLifecycle.java

+
+    @Override
+    public Map<String, String> start() {
+        registry = new GenericContainer<>("jbosstm/lra-coordinator:5.12.0.Final")


Just add a periodic reminder to update the version.

docs/src/main/asciidoc/lra-guide.adoc

cescoffier · 2021-07-05T06:02:00Z

docs/src/main/asciidoc/lra-guide.adoc

+When such a method is called a context is created (if one is not already present) which is passed
+along with subsequent JAX-RS invocations until a method is reached
+which also contains an `@LRA` annotation with an attribute that indicates that the LRA should be
+closed or cancelled. The default is for the LRA to be closed in the same method that started the


I believe this should be written somewhere, maybe using an admonition. Again, readers may not be familiar with the LRA spec (and won't read it).

docs/src/main/asciidoc/lra-guide.adoc

cescoffier · 2021-07-05T06:11:46Z

docs/src/main/asciidoc/lra-guide.adoc

+the method finishes and the `end = true` element on the confirmTrip method forces the LRA
+(started by the bookTrip method) to close the LRA. Note that this end element can
+be placed on any JAX-RS resource (ie one service can start the LRA whilst a different
+service ends it).


Here are some rules I'm trying to apply:

getting started should only contain details to run an example with a small introduction. It should not expect any prior knowledge, and from the beginning to the end should not take more than 15 minutes.

reference contains everything else, more complex example, configuration....

So, typically, from line 164, it should be in the reference guide.
Note that of course, you can cross-link.

cescoffier · 2021-07-05T06:12:35Z

extensions/narayana-lra/runtime/pom.xml

+        <dependency>
+            <groupId>org.jboss.narayana.rts</groupId>
+            <artifactId>lra-proxy-api</artifactId>
+            <scope>compile</scope>


Yes, probably better.

cescoffier · 2021-07-05T06:13:08Z

...ons/narayana-lra/runtime/src/main/java/io/quarkus/narayana/lra/runtime/LRAConfiguration.java

+     * In this version of the extension, a failed coordinator with
+     * LRAs that have not yet finished must be restarted.
+     */
+    @ConfigItem(defaultValue = "http://localhost:8080/lra-coordinator")


Let's do this in an additional PR.

cescoffier · 2021-07-05T06:14:49Z

.../narayana-lra/runtime/src/main/java/io/quarkus/narayana/lra/runtime/NarayanaLRARecorder.java

+@Recorder
+public class NarayanaLRARecorder {
+    public void setConfig(final LRAConfiguration config) {
+        if (System.getProperty(NarayanaLRAClient.LRA_COORDINATOR_URL_KEY) == null) {


Doesn't quarkus.lra.coordinator-url work? IF your test resource returns a Map containing this property it should configure the extension with it.

cescoffier · 2021-07-05T06:15:43Z

integration-tests/narayana-lra/pom.xml

+        </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-resteasy-deployment</artifactId>


You need to remove io.quarkus:quarkus-resteasy, both can't be used at the same time.

mmusgrov · 2021-07-07T08:50:11Z

@cescoffier From my viewpoint it seems that I have resolved each of your objections, ie I don't plan to push any more updates unless you have new input. Can you confirm whether or not you are waiting for me to provide new input?

mmusgrov · 2021-08-04T11:31:30Z

@NahuelOlgiati and @xstefank I updated this PR adding non JAX-RS support.

I included a hack to make the constructors for io.narayana.lra.client.internal.proxy.nonjaxrs.LRAParticipant and LRAParticipantRegistry public. I tried using reflection but could not get it working. I decided not to spend too much time on that since we may (or may not) need to rework the narayana support for non JAX-RS so that it doesn't use CDI extensions.

I will add some more integration tests when I return from my PTO (but my quickstart works).

gsmet · 2021-08-04T11:57:38Z

Just a note that I haven't forgotten about this, it's slowly moving to the top of my list while I go through my backlog and my usual daily tasks.

gsmet · 2021-08-04T11:59:49Z

@mmusgrov can you give me a window when I can work on this PR? It needs a rebase (we don't want merge commits in PRs) and I might have to make a couple of small adjustments here and there. Would Friday work for you?

mmusgrov · 2021-08-04T13:33:46Z

@gsmet Friday works okay for me.
I used the github merge tool for resolving the conflict, I will force push another update ...

quarkus-bot · 2021-08-04T17:12:42Z

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

Failing Jobs - Building `51f000d`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Test failures	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 devtools/cli

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults line 64 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {Downloading JBang...
Installing JBang...
[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] [ERROR] groupId must be specified
[jbang] Run with --verbose for more details
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults(CliProjectJBangTest.java:64)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl....

✖ io.quarkus.cli.CliProjectJBangTest.testCreateCliDefaults line 127 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {[jbang] jbang version 0.78.0
[jbang] System Java version detected as 11
[jbang] System Java version matches requested version 11
[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] Deleting folder /home/runner/.jbang/cache/jars/main.java.af16f5dece403d27e8fc8d88c1488bb303bbbaa9b0cdf91e6455e4d6e21083ff.jar.tmp
[jbang] [ERROR] groupId must be specified
java.lang.IllegalArgumentException: groupId must be specified
	at org.jboss.shrinkwrap.resolver.api.maven.coordinate.MavenDependencies.createExclusion(MavenDependencies.java:174)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromExclusion(MavenConverter.java:81)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromDependency(MavenConverter.java:181)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromDe...

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides line 98 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] [ERROR] groupId must be specified
[jbang] Run with --verbose for more details
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides(CliProjectJBangTest.java:98)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)...

⚙️ JVM Tests - JDK 16 #

📦 devtools/cli

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults line 64 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {Downloading JBang...
Installing JBang...
[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] [ERROR] groupId must be specified
[jbang] Run with --verbose for more details
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults(CliProjectJBangTest.java:64)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl....

✖ io.quarkus.cli.CliProjectJBangTest.testCreateCliDefaults line 127 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {[jbang] jbang version 0.78.0
[jbang] System Java version detected as 16
[jbang] System Java version matches requested version 16
[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] Deleting folder /home/runner/.jbang/cache/jars/main.java.af16f5dece403d27e8fc8d88c1488bb303bbbaa9b0cdf91e6455e4d6e21083ff.jar.tmp
[jbang] [ERROR] groupId must be specified
java.lang.IllegalArgumentException: groupId must be specified
	at org.jboss.shrinkwrap.resolver.api.maven.coordinate.MavenDependencies.createExclusion(MavenDependencies.java:174)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromExclusion(MavenConverter.java:81)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromDependency(MavenConverter.java:181)
	at org.jboss.shrinkwrap.resolver.impl.maven.convert.MavenConverter.fromDe...

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides line 98 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {[jbang] Resolving dependencies...
[jbang] Loading MavenCoordinate [io.quarkus:quarkus-bom:pom:999-SNAPSHOT]
[jbang] [ERROR] groupId must be specified
[jbang] Run with --verbose for more details
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides(CliProjectJBangTest.java:98)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)...

mmusgrov · 2021-08-06T12:06:36Z

@gsmet I think the PR is in good shape but I have some improvements to the manual that could be of help for developers. May I update it or should I wait?

Or, alternatively, I could just say what I wanted to say in a blog (and maybe revisit the manual in a future update).

mmusgrov · 2021-08-06T14:38:50Z

@gsmet The changes look good

gsmet

@mmusgrov I pushed a commit with some small changes.

There's one blocking problem though: we need to figure out what to do with the classes in io.narayana.lra.client.internal.proxy.nonjaxrs (see my comment inline). Maybe ping me on Zulip to discuss it?

gsmet · 2021-08-06T14:38:09Z

...lra/runtime/src/main/java/io/narayana/lra/client/internal/proxy/nonjaxrs/LRAParticipant.java

- * License along with this software; if not, write to the Free
- * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
- */


So... these files are a problem for me:

first, Quarkus is Apache licensed so we need to relicense them under the right license. I have no idea where they come from so I'll let you figure out if it's doable or not;

second are they somehow overriding files in existing jars? It looks like it from what I can see in the other jars. That's definitely something we don't want to do: we don't want a split package and we don't want to override classes this way. I have no idea why you did that so we would need to understand the reason to help you solve this problem.

These files were copied over from the narayana implementation of LRA, I needed access to the constructor which is package private. I have changed them in the narayana repo (jbosstm/narayana#1888) so when we next do a narayana release I will update this extension and remove the two files (LRAParticipant.java and LRAParticipantRegistry.java).

gsmet · 2021-08-06T14:38:42Z

...narayana-lra/runtime/src/main/java/io/quarkus/narayana/lra/runtime/NarayanaLRAProducers.java

@@ -16,7 +15,6 @@ public LRAParticipantRegistry lraParticipantRegistry() {

    @Produces
    public ParticipantProxyResource participantProxyResource() {
-        Object proxy = CDI.current().select(ParticipantProxyResource.class);


This variable is unused. Not sure why you did that or if it's just a leftover?

Thanks (it was a leftover).

gsmet · 2021-08-06T14:38:55Z

...ime/src/main/java/io/narayana/lra/client/internal/proxy/nonjaxrs/LRAParticipantRegistry.java

- * License along with this software; if not, write to the Free
- * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
- */


Same remark here.

Thanks (see my remark).

quarkus-bot · 2021-08-06T19:19:17Z

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

Failing Jobs - Building `dac5303`

Status	Name	Step	Test failures	Logs	Raw logs
✖	Devtools Tests - JDK 11	`Download Maven Repo`	⚠️ Check →	Logs	Raw logs
⌛	Gradle Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 11	`Download Maven Repo`	⚠️ Check →	Logs	Raw logs
✔️	JVM Tests - JDK 16
✖	Maven Tests - JDK 11	`Download Maven Repo`	⚠️ Check →	Logs	Raw logs
✖	Native Tests - Data2	`Download Maven Repo`	⚠️ Check →	Logs	Raw logs

mmusgrov · 2021-08-08T12:48:10Z

@gsmet I'll create a blog at quarkus.io/blog corresponding to the release of the artefact. Can I rely on the extension being listed in the catalog or should tell readers to manually add the dependency io.quarkus:quarkus-narayana-lra to their poms and if so what version should I tell them to use. And who should I submit my blog to?

Addressed

gsmet · 2021-08-09T14:51:48Z

Squashed and rebased. Let's wait for CI and merge.

gsmet · 2021-08-09T14:53:02Z

@mmusgrov when it's merged, it will get available when 2.2 is released. Schedule is here: https://github.com/quarkusio/quarkus/wiki/Release-Planning .

If everything is fine, it should appear automatically in code.quarkus.io, yes.

quarkus-bot · 2021-08-09T18:48:04Z

Failing Jobs - Building `186d7df`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 devtools/cli

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults line 64 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {Downloading JBang...
Error downloading JBang
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppDefaults(CliProjectJBangTest.java:64)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.i...

✖ io.quarkus.cli.CliProjectJBangTest.testCreateCliDefaults line 127 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {Downloading JBang...
Error downloading JBang
},
  system_out: {[DEBUG] Build project with initial parameters: Build [buildOptions=BuildOptions [buildNative=false, clean=true, offline=false, skipTests=false], properties={property=value1, property2=value2, maven.repo.local=/home/runner/.m2/repository}, output=OutputOptions [testMode=true, showErrors=true, verbose=true], params=[]]
/home/runner/work/quarkus/quarkus/devtools/cli/target/test-project/CliProjectJBangTest/code-with-quarkus/jbang --verbose --fresh build -Dproperty=value1 -Dproperty2=value2 -Dmaven.repo.local=/home/runner/.m2/repository -DquarkusRegistryClient=true src/main.java

}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(Asse...

✖ io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides line 98 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 
Expected OK return code. Result:
result: {
  exitCode: {1},
  system_err: {Downloading JBang...
Error downloading JBang
},
  system_out: {}
} ==> 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)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:542)
	at io.quarkus.cli.CliDriver.invokeValidateBuild(CliDriver.java:333)
	at io.quarkus.cli.CliProjectJBangTest.testCreateAppOverrides(CliProjectJBangTest.java:98)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method....

mmusgrov · 2021-08-16T17:10:23Z

@gsmet COMMITTERS.adoc says that if the feature is worth mentioning in the release notes and announcement blog post then the PR requires the release/noteworthy-feature label. Will you add that for me?

@gsmet Also I've written a blog to introduce the extension in adoc format, who should I give it to (there's no repo for blogs) so that it appears at https://quarkus.io/blog?

gsmet · 2021-08-16T19:03:28Z

Merged and added the release/noteworthy-feature.

As for the blog posts, they are there: https://github.com/quarkusio/quarkusio.github.io/tree/develop/_posts
And you need to add your author information here: https://github.com/quarkusio/quarkusio.github.io/blob/develop/_data/authors.yaml

mmusgrov · 2021-08-23T09:48:45Z

Thanks @gsmet - I have have added a PR for the blog. Note that I have just returned from PTO so apologies for its late submission. Is there anyone I should ping to get it added/reviewed?

quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/documentation labels Jun 14, 2021

mmusgrov mentioned this pull request Jun 14, 2021

issue4399 Extension for running Narayana LRA participants #6763

Closed

mmusgrov force-pushed the issue4399-lra-extension branch from 8297860 to 9761d8b Compare June 15, 2021 08:46

mmusgrov force-pushed the issue4399-lra-extension branch from 9761d8b to 7a506d9 Compare June 15, 2021 09:05

quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Jun 15, 2021

mmusgrov force-pushed the issue4399-lra-extension branch from 7a506d9 to ba32fc2 Compare June 15, 2021 13:51

NahuelOlgiati reviewed Jun 16, 2021

View reviewed changes

...na-lra/deployment/src/main/java/io/quarkus/narayana/lra/deployment/NarayanaLRAProcessor.java Show resolved Hide resolved

NahuelOlgiati reviewed Jun 16, 2021

View reviewed changes

xstefank reviewed Jun 18, 2021

View reviewed changes

mmusgrov force-pushed the issue4399-lra-extension branch from ba32fc2 to 1203f89 Compare June 21, 2021 17:37

xstefank approved these changes Jun 22, 2021

View reviewed changes

NahuelOlgiati approved these changes Jun 22, 2021

View reviewed changes

cescoffier self-requested a review June 25, 2021 12:28

cescoffier requested changes Jun 29, 2021

View reviewed changes

cescoffier previously requested changes Jul 5, 2021

View reviewed changes

mmusgrov force-pushed the issue4399-lra-extension branch from 082a342 to 063963d Compare July 5, 2021 18:08

ochaloup mentioned this pull request Jul 19, 2021

[JBTM-3427] split arq and arq.quarkus profiles jbosstm/narayana#1881

Merged

mmusgrov force-pushed the issue4399-lra-extension branch 3 times, most recently from f6ea1b9 to 812179d Compare August 4, 2021 11:17

mmusgrov force-pushed the issue4399-lra-extension branch from 69e01fa to 51f000d Compare August 4, 2021 13:35

mmusgrov force-pushed the issue4399-lra-extension branch from 51f000d to 1ade264 Compare August 4, 2021 17:45

gsmet force-pushed the issue4399-lra-extension branch from 5e715fc to dac5303 Compare August 6, 2021 14:43

gsmet requested changes Aug 6, 2021

View reviewed changes

issue4399 Extension for running Narayana LRA participants

186d7df

gsmet force-pushed the issue4399-lra-extension branch from dac5303 to 186d7df Compare August 9, 2021 14:50

gsmet approved these changes Aug 9, 2021

View reviewed changes

gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 9, 2021

gsmet merged commit ca53ffc into quarkusio:main Aug 16, 2021

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 16, 2021

quarkus-bot bot added this to the 2.2 - main milestone Aug 16, 2021

gsmet added the release/noteworthy-feature label Aug 16, 2021

mmusgrov mentioned this pull request Aug 23, 2021

LRA extension blog post quarkusio/quarkusio.github.io#1105

Merged


		== Introduction

		The LRA (short for Long Running Action) participant extension is useful in microservice based

	The LRA (short for Long Running Action) participant extension is useful in microservice based
	The LRA (short for Long Running Actions) participant extension is useful in microservice based

Extension for running Narayana LRA participants #17903

Extension for running Narayana LRA participants #17903

Conversation

mmusgrov commented Jun 14, 2021

quarkus-bot bot commented Jun 14, 2021 • edited Loading

Failing Jobs - Building 8297860

quarkus-bot bot commented Jun 15, 2021 • edited Loading

Failing Jobs - Building 9761d8b

quarkus-bot bot commented Jun 15, 2021 • edited Loading

Failing Jobs - Building 7a506d9

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/narayana-lra

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmusgrov commented Jun 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xstefank left a comment

Choose a reason for hiding this comment

quarkus-bot bot commented Jun 22, 2021 • edited Loading

Failing Jobs - Building 082a342

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 devtools/cli

⚙️ JVM Tests - JDK 16 #

📦 devtools/cli

📦 integration-tests/grpc-hibernate-reactive

mmusgrov commented Jun 24, 2021

cescoffier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmusgrov Jul 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quarkus-bot bot commented Jun 14, 2021 •

edited

Loading

Failing Jobs - Building `8297860`

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `9761d8b`

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `7a506d9`

quarkus-bot bot commented Jun 22, 2021 •

edited

Loading

Failing Jobs - Building `082a342`

mmusgrov Jul 2, 2021 •

edited

Loading