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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Custom cockroach binary via env variable #84

Merged
merged 1 commit into from
Aug 7, 2020
Merged

Custom cockroach binary via env variable #84

merged 1 commit into from
Aug 7, 2020

Conversation

georgysavva
Copy link
Contributor

This adds the ability to specify a custom cockroach binary via COCKROACH_BINARY env variable.
Existing --cockroach-binary flag has more priority than the new env variable.
See: #68

georgysavva
georgysavva commented Jul 29, 2020
View reviewed changes
testserver/testserver.go Outdated Show resolved Hide resolved
testserver/testserver.go Outdated Show resolved Hide resolved
@rafiss
Copy link
Contributor

rafiss commented Aug 5, 2020

LGTM! thank you. would you mind rebasing? master has the whitespace issue fixed and the logic was refactored.

@rafiss rafiss self-requested a review August 5, 2020 19:25
@georgysavva georgysavva force-pushed the binary-from-env-var branch from fb4f3cf to f84584f Compare August 6, 2020 08:02
@georgysavva
Copy link
Contributor Author

Done.

@georgysavva
Copy link
Contributor Author

georgysavva commented Aug 6, 2020 •
edited
Loading

Tests are failing (the same is in the master):

=== RUN   TestRunServer/SecureTenant
=== CONT  TestRunServer
    testserver_test.go:44: dial tcp 127.0.0.1:37305: connect: connection refused

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rafiss
Copy link
Contributor

rafiss commented Aug 6, 2020

It looks like that test is broken against cockroachdb master. Will skip for now, and I created #87

@rafiss
Copy link
Contributor

rafiss commented Aug 7, 2020

I merged #88 which skips that test. Sorry for the hassle, but could you try rebasing again?

@georgysavva georgysavva force-pushed the binary-from-env-var branch from f84584f to d21fbac Compare August 7, 2020 14:19
@georgysavva
Copy link
Contributor Author

Sure, done!

rafiss
rafiss approved these changes Aug 7, 2020
View reviewed changes
Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm! thanks

@rafiss rafiss merged commit 4a99d43 into cockroachdb:master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafiss rafiss rafiss approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@georgysavva @rafiss @cockroach-teamcity