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.

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

chore(content/update): Refactor and Amend Weld Message #1669

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

psavidis
Copy link
Contributor

@psavidis psavidis commented Aug 6, 2024

Content

  • Refactor Weld Informative message to be more clearly phrased
  • Add a message to state the workaround limitations (cross-archive references)

Related-to: camunda/camunda-bpm-platform#4499

@psavidis psavidis self-assigned this Aug 6, 2024
@psavidis psavidis changed the title 4499 add weld cross archive bean warning chore(content/update): Refactor and Amend Weld Message Aug 6, 2024
@psavidis psavidis requested a review from tasso94 August 6, 2024 10:23
Copy link
Member

@tasso94 tasso94 left a comment

Choose a reason for hiding this comment

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

Where can I find the installation guide hint?

@psavidis
Copy link
Contributor Author

psavidis commented Aug 6, 2024

Where can I find the installation guide hint?

❓ I couldn't find a relevant section to add the hint. Where do you think the weld info could fit in the subcategories of the Tomcat Installation guide ?

@psavidis
Copy link
Contributor Author

psavidis commented Aug 6, 2024

Update

After applying the code review suggestion the formatting of the text didn't feel right.
I've added a commit to add new line characters to make the formatting more structurally appealing.

Here are the Before / After Screenshots to ease up the review process:

Before Screenshot 2024-08-06 at 3 48 35 PM
After Screenshot 2024-08-06 at 3 48 07 PM

PS: The same change was applied to the Patch Level Update Section

@psavidis psavidis requested a review from tasso94 August 6, 2024 12:53
@tasso94
Copy link
Member

tasso94 commented Aug 6, 2024

❓ I couldn't find a relevant section to add the hint. Where do you think the weld info could fit in the subcategories of the Tomcat Installation guide ?

Let's glue a note box to the installation guide where we repeat the information for Tomcat 10.

Screenshot 2024-08-06 at 14 56 00

Maybe something along the following lines:

{{< note title="Known Limitations in Tomcat 10" class="info" >}}

**<h4>Weld Class Loading Issues</h4>**

In deployment scenarios involving one or more process applications with managed beans, classloading issues may occur if the WELD library is directly embedded in the WAR's `/WEB-INF/lib` folder.
To resolve this, move the weld library away from the war and place it into the `$CATALINA_HOME/lib` folder.

The above fix is not guaranteed to work for cases with bean references between WAR deployments (WAR A referencing a bean from WAR B). 

The following test scenarios fail on Tomcat 10:

* [CallActivityContextSwitchTest](https://github.com/camunda/camunda-bpm-platform/blob/f37877b822dabcbf3cee5806bd5833d18cdcb543/qa/integration-tests-engine/src/test/java/org/camunda/bpm/integrationtest/functional/context/CallActivityContextSwitchTest.java)
* [CdiBeanCallActivityResolutionTest](https://github.com/camunda/camunda-bpm-platform/blob/f37877b822dabcbf3cee5806bd5833d18cdcb543/qa/integration-tests-engine/src/test/java/org/camunda/bpm/integrationtest/functional/cdi/CdiBeanCallActivityResolutionTest.java)
{{< /note >}}

Should be part of the 7.21 installation guide as well.

@psavidis
Copy link
Contributor Author

psavidis commented Aug 6, 2024

❓ I couldn't find a relevant section to add the hint. Where do you think the weld info could fit in the subcategories of the Tomcat Installation guide ?

Let's glue a note box to the installation guide where we repeat the information for Tomcat 10.

Screenshot 2024-08-06 at 14 56 00

Maybe something along the following lines:

{{< note title="Known Limitations in Tomcat 10" class="info" >}}

**<h4>Weld Class Loading Issues</h4>**

In deployment scenarios involving one or more process applications with managed beans, classloading issues may occur if the WELD library is directly embedded in the WAR's `/WEB-INF/lib` folder.
To resolve this, move the weld library away from the war and place it into the `$CATALINA_HOME/lib` folder.

The above fix is not guaranteed to work for cases with bean references between WAR deployments (WAR A referencing a bean from WAR B). 

The following test scenarios fail on Tomcat 10:

* [CallActivityContextSwitchTest](https://github.com/camunda/camunda-bpm-platform/blob/f37877b822dabcbf3cee5806bd5833d18cdcb543/qa/integration-tests-engine/src/test/java/org/camunda/bpm/integrationtest/functional/context/CallActivityContextSwitchTest.java)
* [CdiBeanCallActivityResolutionTest](https://github.com/camunda/camunda-bpm-platform/blob/f37877b822dabcbf3cee5806bd5833d18cdcb543/qa/integration-tests-engine/src/test/java/org/camunda/bpm/integrationtest/functional/cdi/CdiBeanCallActivityResolutionTest.java)
{{< /note >}}

Should be part of the 7.21 installation guide as well.

Applied the suggested change. Will follow up with the 7.21 backport once this PR is finalised and merged.

@psavidis psavidis requested a review from tasso94 August 7, 2024 06:17
@psavidis psavidis requested a review from tasso94 August 7, 2024 08:07
- Tomcat Installation Guide
- Migration Guide
- Patch Level Section

Co-authored-by: tasso94 <3015690+tasso94@users.noreply.github.com>
Related-to: camunda/camunda-bpm-platform#4499
@psavidis psavidis force-pushed the 4499-add-weld-cross-archive-bean-warning branch from 6f95ad8 to 93534cc Compare August 8, 2024 06:16
Copy link
Member

@tasso94 tasso94 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 now 🙂 👍 I replaced the word fix with workaround.

@psavidis psavidis merged commit 67e2e3c into master Aug 8, 2024
1 check passed
@psavidis psavidis deleted the 4499-add-weld-cross-archive-bean-warning branch August 8, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants