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

Fixes: 209: introduce SERF_RPC_ADDR to do the same as -rpc-addr #210

Merged
merged 4 commits into from
May 4, 2014
Merged

Fixes: 209: introduce SERF_RPC_ADDR to do the same as -rpc-addr #210

merged 4 commits into from
May 4, 2014

Conversation

lalyos
Copy link
Contributor

@lalyos lalyos commented May 3, 2014

No description provided.

@ryanuber
Copy link
Member

ryanuber commented May 3, 2014

Maybe the command-line args should take precedence over the environment. Might be confusing to see serf agent -rpc-addr=... in the process table but actually having a different address in use. Command line args also take precedence over config files. Other than that, LGTM.

@mitchellh
Copy link
Contributor

Agree with @ryanuber, otherwise LGTM

lalyos added 3 commits May 4, 2014 06:54
@lalyos
Copy link
Contributor Author

lalyos commented May 4, 2014

@ryanuber you're right, command-line args should take precendence.

Something is strange with travis:

I did the change, and was surprised by failing travis, so I

  • I tested my changes in via crosbymichael's golang docker image and tests passed
  • i pushed the code into github, and travis failed
  • then by checking that travis uses tip go version, i explicitly set 1.2.1 and travis passed
  • than changed back to tip and travis .. passed

maybe the tests has some raceconditions?

@ryanuber
Copy link
Member

ryanuber commented May 4, 2014

@lalyos Serf does have a bit of trouble with operations that involve timing while running the tests under travis CI. This has gotten a bit better over the last few months but can still be a little unpredictable. Your changes look good. Thanks!

ryanuber added a commit that referenced this pull request May 4, 2014
Fixes: 209: introduce SERF_RPC_ADDR to do the same as -rpc-addr
@ryanuber ryanuber merged commit 3a9d851 into hashicorp:master May 4, 2014
@lalyos lalyos deleted the 209-add-SERF_RPC_ADDR branch May 4, 2014 18:13
lalyos added a commit to sequenceiq/consul that referenced this pull request Dec 16, 2014
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.

3 participants