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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

apache-kafka: Fix, update and adjust JRE/JVM #98446

Merged
merged 5 commits into from Mar 10, 2021
Merged

apache-kafka: Fix, update and adjust JRE/JVM #98446

merged 5 commits into from Mar 10, 2021

Conversation

srhb
Copy link
Contributor

@srhb srhb commented Sep 22, 2020 •
edited

Most Kafka versions were broken by the recent jdk/jre changes in #89731 but it needed some cleanup work anyway. 馃槃

I've enriched the package versions with relevant JRE compatibility, and passed it through so that the module can take advantage of it to set a jre by default (new option) that corresponds in the service.

Also remove the default jvmOptions to avoid having to pass these through in version-compatible permutations. Users should adjust them according to the version they use, so add a release note that warns about this.

Also adds the latest kafka 2.6, sets it to the default apacheKafka and adjust the Scala version used for 2.5.X according to upstream recommendations.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from ragnard September 22, 2020 07:38
@srhb
Copy link
Contributor Author

srhb commented Sep 23, 2020

Force pushed to use the headless variants, the default ones pull in a ton of X deps.

@GTrunSec
Copy link
Contributor

GTrunSec commented Oct 12, 2020 •
edited

@srhb, please resolve the conflict. furthermore, about the JVM option. #100115 thanks @NeQuissimus are you maintainter here?

@srhb srhb force-pushed the fix-kafka branch 2 times, most recently from 717bc06 to 4af0b62 Compare October 13, 2020 05:32
@srhb
Copy link
Contributor Author

srhb commented Oct 13, 2020

Conflicts resolved. 馃檪

@NeQuissimus
Copy link
Member

@srhb LGTM but can you add 2.6 to the test suite, please? :)

@srhb
Copy link
Contributor Author

srhb commented Oct 13, 2020

Thought I did already in a912e93276835b377dd0ad02fcfebf9e5e5f8cfe - did I miss another place?

@NeQuissimus
Copy link
Member

Oh wow, I must be blind :D

@bbigras
Copy link
Contributor

bbigras commented Mar 9, 2021

Any progress on this?

@NeQuissimus
Copy link
Member

This should be good to go after a rebase

@srhb
Copy link
Contributor Author

srhb commented Mar 9, 2021

I'm sorry, I forgot all about this. I'll fix it up now.

@bbigras
Copy link
Contributor

bbigras commented Mar 9, 2021

I'm sorry, I forgot all about this. I'll fix it up now.

Hey no worries :) Thank you very much for the PR and the quick rebase.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/356

@srhb
Copy link
Contributor Author

srhb commented Mar 10, 2021 •
edited

I'm going to go ahead and merge once ofborg passes since there's some approval at least, and I don't want to play whack-a-mole with the release note merge conflicts. :-)

@srhb srhb merged commit 33d6e71 into NixOS:master Mar 10, 2021
@srhb srhb deleted the fix-kafka branch March 10, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NeQuissimus NeQuissimus NeQuissimus approved these changes

@ragnard ragnard Awaiting requested review from ragnard

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@srhb @GTrunSec @NeQuissimus @bbigras @nixos-discourse @ryantm @SuperSandro2000