-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add az acs kubernetes browse
and also open a web browser.
#1327
Conversation
This will be an awesome helper command. Thanks for the quick response! |
c2aeb62
to
460838d
Compare
""" | ||
Wrapper on the 'kubectl proxy' command, for consistency with 'az dcos brows' | ||
""" | ||
logger.info('Proxy running on 127.0.0.1:8001/ui') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be logger.warning
I believe.
logger.info
will only be should to the user if they add the --verbose
or --debug
flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
||
def k8s_browse(disable_browser=False): | ||
""" | ||
Wrapper on the 'kubectl proxy' command, for consistency with 'az dcos brows' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: browse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
t.daemon = True | ||
t.start() | ||
|
||
def k8s_browse(disable_browser=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add help for the disable_browser
param.
You can modify _params.py. Could add something like this:
register_cli_argument('acs', 'disable_browser', help='Do not open browser after command execution')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By passing in the first argument as 'acs', it'll add help for the DCOS browse command below as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@@ -61,6 +87,8 @@ def dcos_browse(name, resource_group_name): | |||
proxy.set_http_proxy('127.0.0.1', local_port) | |||
logger.info('Proxy running on 127.0.0.1:%s', local_port) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be warning instead of info also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
460838d
to
e47b96f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* Azure/master: Add `az acs kubernetes browse` and also open a web browser. (Azure#1327)
Fixes #1321
@lostintangent