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

roachtest: fix import/tpcc/warehouses=1000/nodes=4 #37915

Merged
merged 1 commit into from
Jun 3, 2019

Conversation

danhhz
Copy link
Contributor

@danhhz danhhz commented May 29, 2019

In #37726, I switched a few places use the workload fixtures import in
the cockroach cli so the version string would match between the
fixtures import command and whatever was producing the data, but I
should have left this one alone. It's using --csv-server so, in
contrast to the other places I fixed that that PR, in this case the
workload binary is both ends. (--csv-server causes cockroach to read
csv data from an http server inside the standalone workload command.)

This broke because fixtures import works with the --csv-server flag
on master, but that hasn't been backported to anything else, so the
workload built into the cockroach cli doesn't know about that flag for
fixtures import. My smoke tests didn't catch this because I was using a
cockroach binary built from master.

Hopefully this is the last of the fallout.

Touches #37371

Release note: None

In cockroachdb#37726, I switched a few places use the `workload fixtures import` in
the cockroach cli so the version string would match between the
`fixtures import` command and whatever was producing the data, but I
should have left this one alone. It's using `--csv-server` so, in
contrast to the other places I fixed that that PR, in this case the
workload binary is both ends. (`--csv-server` causes cockroach to read
csv data from an http server inside the standalone workload command.)

This broke because `fixtures import` works with the `--csv-server` flag
on master, but that hasn't been backported to anything else, so the
workload built into the cockroach cli doesn't know about that flag for
fixtures import. My smoke tests didn't catch this because I was using a
cockroach binary built from master.

Hopefully this is the last of the fallout.

Touches cockroachdb#37371

Release note: None
@danhhz danhhz requested a review from tbg May 29, 2019 16:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@danhhz
Copy link
Contributor Author

danhhz commented May 29, 2019

TFTR!

bors r=tbg

@danhhz
Copy link
Contributor Author

danhhz commented May 30, 2019

Seems like bors dropped this

bors r=tbg

@danhhz
Copy link
Contributor Author

danhhz commented Jun 3, 2019

bors r=tbg

craig bot pushed a commit that referenced this pull request Jun 3, 2019
37915: roachtest: fix import/tpcc/warehouses=1000/nodes=4 r=tbg a=danhhz

In #37726, I switched a few places use the `workload fixtures import` in
the cockroach cli so the version string would match between the
`fixtures import` command and whatever was producing the data, but I
should have left this one alone. It's using `--csv-server` so, in
contrast to the other places I fixed that that PR, in this case the
workload binary is both ends. (`--csv-server` causes cockroach to read
csv data from an http server inside the standalone workload command.)

This broke because `fixtures import` works with the `--csv-server` flag
on master, but that hasn't been backported to anything else, so the
workload built into the cockroach cli doesn't know about that flag for
fixtures import. My smoke tests didn't catch this because I was using a
cockroach binary built from master.

Hopefully this is the last of the fallout.

Touches #37371

Release note: None

Co-authored-by: Daniel Harrison <[email protected]>
@craig
Copy link
Contributor

craig bot commented Jun 3, 2019

Build succeeded

@craig craig bot merged commit 7c21bfe into cockroachdb:master Jun 3, 2019
@danhhz danhhz deleted the roachtest branch June 5, 2019 15:17
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.

3 participants