Skip to content

nixosTests.cockroachdb: port to python#73934

Merged
worldofpeace merged 6 commits intoNixOS:masterfrom
flokli:nixos-test-port-cockroachdb
Apr 19, 2020
Merged

nixosTests.cockroachdb: port to python#73934
worldofpeace merged 6 commits intoNixOS:masterfrom
flokli:nixos-test-port-cockroachdb

Conversation

@flokli
Copy link
Copy Markdown
Member

@flokli flokli commented Nov 22, 2019

Motivation for this change

#72828

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 nix-review --run "nix-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.
Notify maintainers

cc @tfc

@flokli flokli requested a review from fpletz November 22, 2019 20:02
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 22, 2019
@flokli flokli force-pushed the nixos-test-port-cockroachdb branch 2 times, most recently from 8add180 to fb124c5 Compare November 22, 2019 21:31
@ofborg ofborg bot added the 8.has: module (update) This PR changes an existing module in `nixos/` label Nov 22, 2019
@worldofpeace
Copy link
Copy Markdown
Contributor

@GrahamcOfBorg test cockroachdb

@worldofpeace
Copy link
Copy Markdown
Contributor

Seeing sort of failure only on x86_64-linux

/nix/store/fbqss54p4xbj10zi93ywlikgij15adhr-nixos-test-driver/bin/.nixos-test-driver-wrapped:639: DeprecationWarning: invalid escape sequence '\@'
  line = _line.decode("unicode_escape").replace("\r", "").rstrip()
node1 # Error: parse postgresql://root\@192.168.1.1:26257?sslmode=disable: net/url: invalid userinfo
node1: output:
error: command `cockroach workload init bank 'postgresql://root\@192.168.1.1:26257?sslmode=disable'` failed (exit code 1)

Copy link
Copy Markdown
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

I believe this is what's causing that issue, this doesn't even need to be escaped.
Tested locally.

@flokli flokli force-pushed the nixos-test-port-cockroachdb branch from fb124c5 to 1c9be5c Compare November 23, 2019 14:09
@flokli
Copy link
Copy Markdown
Member Author

flokli commented Nov 23, 2019

Argh, 🤦‍♂️. Thanks for spotting that!

@flokli flokli force-pushed the nixos-test-port-cockroachdb branch 2 times, most recently from e9bd08b to 2419ecc Compare November 23, 2019 15:09
@flokli
Copy link
Copy Markdown
Member Author

flokli commented Nov 23, 2019

@GrahamcOfBorg test cockroachdb

@flokli
Copy link
Copy Markdown
Member Author

flokli commented Nov 23, 2019

For some reason, this still doesn't converge properly here - even though it kinda succeeds in the perl version. 🤔

@flokli
Copy link
Copy Markdown
Member Author

flokli commented Nov 23, 2019

@GrahamcOfBorg test cockroachdb

@flokli flokli force-pushed the nixos-test-port-cockroachdb branch from 4fec4bb to 0734591 Compare December 6, 2019 09:24
@flokli
Copy link
Copy Markdown
Member Author

flokli commented Dec 6, 2019

@GrahamcOfBorg test cockroachdb

@tfc
Copy link
Copy Markdown
Contributor

tfc commented Jan 14, 2020

@flokli what's the state of this?

@flokli
Copy link
Copy Markdown
Member Author

flokli commented Jan 14, 2020 via email

@tfc tfc mentioned this pull request Feb 13, 2020
10 tasks
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

IIUIC, this needs , between the strings, otherwise they're concatenated.

Copy link
Copy Markdown
Member Author

@flokli flokli Apr 2, 2020

Choose a reason for hiding this comment

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

nice catch! However, I still don't get the servers to sync together. It did eventually succeed!

@flokli flokli force-pushed the nixos-test-port-cockroachdb branch from 0734591 to fe6dd71 Compare April 2, 2020 22:35
@flokli
Copy link
Copy Markdown
Member Author

flokli commented Apr 2, 2020

I rebased this on latest master.

@flokli
Copy link
Copy Markdown
Member Author

flokli commented Apr 2, 2020

@GrahamcOfBorg test cockroachdb

@flokli
Copy link
Copy Markdown
Member Author

flokli commented Apr 2, 2020

@tfc this test works now, PTAL.

@worldofpeace worldofpeace requested a review from tfc April 8, 2020 03:42
@worldofpeace worldofpeace merged commit f882896 into NixOS:master Apr 19, 2020
@flokli flokli deleted the nixos-test-port-cockroachdb branch April 19, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants