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

Set up KAFKA environment variables for the service execution #54

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

rpelisse
Copy link
Collaborator

No description provided.

@rpelisse
Copy link
Collaborator Author

@rmarting Can you review this PR (and maybe test it a bit)?

Thanks!

@rpelisse rpelisse merged commit f515717 into main Jul 19, 2023
1 check passed
@@ -27,4 +27,6 @@
server_group: "{{ amq_streams_connect_group | default(omit) }}"
server_java_opts: "{{ amq_streams_connect_java_opts | default('') }}"
server_java_heap_opts: "{{ amq_streams_connect_java_heap_opts | default('') }}"
server_java_performance_opts: "{{ amq_streams_connect_java_performance_opts | default('') }}"
server_java_gc_log_opts: "{{ amq_streams_connect_java_java_gc_log_opts | default('') }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am missing the definiton for the jmx, something similar to:

server_java_jmx_opts: "{{ amq_streams_connect_java_jmx_opts | default('') }}"

Is there a reasson for that? That variable is declared for Broker and Zookeeper, but not for the Kafka Connect.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rpelisse Tested and verified everything successfully. Only a minor details for the Kafka Connect service. Added a comment about that. ✔️

For the rest of the implementation. LGTM.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another comment, I found that the server_log4j_opts variable is not declared to be used for the service task. I mean, I don't see how to identify a value for that variable.

Am I missing something?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rpelisse I think you missed this comment! As I can't find the way to declare that variable for each component. Am I missing sth? Thanks

@rpelisse rpelisse deleted the ISSUE_53 branch July 24, 2023 11:48
@rpelisse rpelisse added the minor_changes New parameters added to modules, or non-breaking behavior changes to existing parameters; no bugfix label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor_changes New parameters added to modules, or non-breaking behavior changes to existing parameters; no bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants