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

cassandra 3.11.3 -> 3.11.4 #58025

Merged
merged 1 commit into from Jul 17, 2019
Merged

cassandra 3.11.3 -> 3.11.4 #58025

merged 1 commit into from Jul 17, 2019

Conversation

contrun
Copy link
Contributor

@contrun contrun commented Mar 21, 2019

Motivation for this change

update cassandra

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@nlewo
Copy link
Member

nlewo commented Mar 23, 2019

@GrahamcOfBorg test cassandra

@aanderse
Copy link
Member

aanderse commented Apr 8, 2019

Looks like the cassandra test may have been removed from nixos/tests/all-tests.nix at some point? @contrun do you mind adding the cassandra test back to this file so we can run tests here?

@contrun
Copy link
Contributor Author

contrun commented Apr 8, 2019

@aanderse Looks like the only file in the nixos/tests directory contains cassandra is nixos/tests/cassandra.nix. I tried to search cassandra with git grep cassandra $(git rev-list --all -- nixos/tests/) -- nixos/tests/ | awk -F: '{print $2}' | sort | uniq.

@aanderse
Copy link
Member

aanderse commented Apr 8, 2019

@contrun pardon my poor choice of wording. The actual test remains, but the test is not actually executed by the bot because a reference to the test was removed (or never existed?) in all-tests.nix.

Can you please create an additional commit adding the test back into all-tests.nix?

@contrun contrun force-pushed the contrun-patch-1 branch 2 times, most recently from 5516bf5 to b9a7345 Compare April 8, 2019 11:24
@contrun
Copy link
Contributor Author

contrun commented Apr 8, 2019

@aanderse Added that. But I am not familiar with the testing process. Not sure if it will work.

@aanderse
Copy link
Member

aanderse commented Apr 8, 2019

I've been running the test from the master branch locally for ~10 minutes now and no success.

@cransom as the listed maintainer of the cassandra package do you have any information which could help us?

@aanderse
Copy link
Member

aanderse commented Apr 8, 2019

@GrahamcOfBorg test cassandra

@aanderse
Copy link
Member

aanderse commented Apr 9, 2019

@JohnAZoidberg as someone interested in cassandra do you have any insight into why the test is not succeeding?

@JohnAZoidberg
Copy link
Member

Seems like you can't have a node as its own seed when it's listening on an interface instead of an IP.
At least the way it's done in the current test doesn't work and I can't figure out any other way.

I'll push a fix to my PR from yesterday that improves the config file: #59179

@aanderse
Copy link
Member

aanderse commented May 2, 2019

@contrun Any chance I can get you to leave a review and approval on #59179 if everything looks good to you? Bonus points if you test out the changes in your own configuration and mention results. Once #59179 is merged it seems like you could rebase this PR on master and we could quickly and confidently merge this. Sorry for the hold up so far.

@JohnAZoidberg
Copy link
Member

And if you want you can upgrade the other supported versions we have to their new point release that came out on the same day: https://github.com/apache/cassandra/releases

@aanderse
Copy link
Member

This PR can be merged now as there is no blocker anymore, right? 100% relying on you @JohnAZoidberg to tell me whether to push the merge button.

@contrun Are you able to rebase on master and possibly update other versions as requested by @JohnAZoidberg?

also add test to all-test.nix
@JohnAZoidberg
Copy link
Member

@GrahamcOfBorg test cassandra

Copy link
Member

@JohnAZoidberg JohnAZoidberg left a comment

Choose a reason for hiding this comment

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

The tests take too long for OfBorg. They run fine on my machine.

@aanderse
Copy link
Member

@GrahamcOfBorg build cassandra

@aanderse
Copy link
Member

Merging solely based on the good word of @JohnAZoidberg.

@aanderse aanderse merged commit dd1d0b9 into NixOS:master Jul 17, 2019
@contrun contrun deleted the contrun-patch-1 branch July 18, 2019 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants