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

Adding devops and acs modules to cli. #1108

Merged
merged 4 commits into from
Oct 21, 2016
Merged

Adding devops and acs modules to cli. #1108

merged 4 commits into from
Oct 21, 2016

Conversation

sanar-microsoft
Copy link
Member

@sanar-microsoft sanar-microsoft commented Oct 20, 2016 •
edited
Loading

Commands added:

  1. acs dcos browse: to create ssh proxy to the acs cluster.
  2. devops release create: to build and deploy services to acs cluster.
  3. devops release list: to list current releases in the acs cluster.
  4. devops build create: to build from a repo in the acs cluster.

@azurecla
Copy link

Hi @sanar-microsoft, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (sanar). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

@derekbekoe derekbekoe mentioned this pull request Oct 21, 2016
Closed
derekbekoe
derekbekoe approved these changes Oct 21, 2016
View reviewed changes
Copy link
Member

@derekbekoe derekbekoe left a comment

Choose a reason for hiding this comment

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

LGTM but I have one comment

src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/acs_client.py
if e.errno == errno.ENOENT:
raise Exception('Remote file "{}" does not exist'.format(remote_file))
else:
assert False, 'Error accessing remote file {}'.format(remote_file)
Copy link
Member

Choose a reason for hiding this comment

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

Why would you need to assert False here

Copy link
Member Author

Choose a reason for hiding this comment

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

This was to trigger the assert. I can change this to raise Exception('Error accessing remote file {}'.format(remote_file))

Copy link
Member

Choose a reason for hiding this comment

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

Yes that sounds better than using assert.

@derekbekoe
Copy link
Member

Merging PR.

@derekbekoe derekbekoe merged commit de089bf into Azure:master Oct 21, 2016
@YuzorMa YuzorMa added Container Instances az container ACS az acs/aks/openshift labels Nov 30, 2016
@pahaz pahaz mentioned this pull request Jun 16, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekbekoe derekbekoe derekbekoe approved these changes

Assignees
No one assigned
Labels
ACS az acs/aks/openshift cla-not-required Container
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sanar-microsoft @azurecla @derekbekoe @YuzorMa @haroldrandom