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

(Mac OS X) circuit ls command not working with CIRCUIT_DISCOVER #15

Open
traviskaufman opened this issue Oct 26, 2014 · 2 comments
Open

Comments

@traviskaufman
Copy link

Here's what I've done:

  • Enabled Multicasting on the loopback if (lo0) and my standard en0 interface
  • In a terminal window:
circuit start -if lo0 -discover 228.1.1.2:7711
  • In a separate terminal window
CIRCUIT_DISCOVER=228.1.1.2:7711 circuit ls /...

In this case, circuit ls simply hangs.

However if I do this:

228.1.1.2:7711 circuit ls -discover '228.1.1.2:7711' /...

it works as expected.

Interestingly, it also works if I do this:

CIRCUIT_DISCOVER=228.1.1.2:7711 circuit ls -discover '' /...

I noticed in util.go dial() func that there's a switch statement that checks for a discover flag before looking at CIRCUIT_DISCOVER. Could it be that a default argument is being supplied to -discover that is being looked at instead of the env variable?

Environment:

screen shot 2014-10-26 at 4 14 05 pm

Let me know if you need any more info from me. Thanks!

@maymounkov
Copy link

The command-line argument overwrites the environment. Standard convention.
Does this make sense?

On 26 October 2014 16:20, Travis Kaufman [email protected] wrote:

Here's what I've done:

  • Enabled Multicasting on the loopback if (lo0) and my standard en0
    interface
  • In a terminal window:

circuit start -if lo0 -discover 228.1.1.2:7711

  • In a separate terminal window

CIRCUIT_DISCOVER=228.1.1.2:7711 circuit ls /...

In this case, circuit ls simply hangs. However if I do this:

228.1.1.2:7711 circuit ls -discover '228.1.1.2:7711' /...

Interestingly, it also works if I do this:

CIRCUIT_DISCOVER=228.1.1.2:7711 circuit ls -discover '' /...

I noticed in ls.go that there's a switch statement that checks for a
discover flag before looking at CIRCUIT_DISCOVER. Could it be that a
default argument is being supplied to -discover that is being looked at
instead of the env variable?

Environment:

[image: screen shot 2014-10-26 at 4 14 05 pm]
https://cloud.githubusercontent.com/assets/1185269/4784271/d7e877a6-5d4c-11e4-916a-315603ced8b0.png

Let me know if you need any more info from me. Thanks!


Reply to this email directly or view it on GitHub
#15.

@jsipprell
Copy link

The command-line argument overwrites the environment. Standard convention.
Does this make sense?

You are misunderstanding what the original issue opener is saying.

In main.go you initialize the github.com/codegangsta/cli global context flags, including --discover with a default value. Then, in util.go you use a switch statement to check both the --discover flag and the CIRCUIT_DISCOVER environment variable in that order. Because you use ctx.String() to check for "discover" you will always get a value before the environment variable is checked. This happens because github.com/codegangsta/cli returns the default value if the user has not specified the flag. In other words, it is not possible for the environment variable value to be used because there is always a non-zero value string available for "discover". This happens irrespective of platform, it's entirely an incorrect usage of the (modern) cli package.

The fix for this is to either use the in-built github.com/codegangsta/cli environment variable support or use x.IsSet() in your switch instead of x.String() in order to detect if the user has actually specified the flag (you would then use x.String() to recover the value inside the case selector).

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

No branches or pull requests

3 participants