Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Get sub from azure CLI when not provided #3519

Merged
merged 2 commits into from
Jul 23, 2018

Conversation

cpuguy83
Copy link
Member

What this PR does / why we need it:

When a user doesn't provide a subscription explicitly, try to use the selected subscription from azure CLI.

Special notes for your reviewer:

Not sure if it's overkill to first check what the selected cloud is vs just always looking for AzureCloud.

If applicable:

  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

Read subscription ID from azure CLI config when not explicitly provided 

@@ -118,3 +120,15 @@ func AcceleratedNetworkingSupported(sku string) bool {
}
return true
}

// GetHomeDir attempts to get the home dir from env
func GetHomeDir() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

A unit test for this new helper function would be great

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'm just not sure how to adequately test it without basically putting the same exact logic in the test itself.

@acs-bot
Copy link

acs-bot commented Jul 23, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cpuguy83
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: cecilerobertmichon

If they are not already assigned, you can assign the PR to them by writing /assign @cecilerobertmichon in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov
Copy link

codecov bot commented Jul 23, 2018

Codecov Report

Merging #3519 into master will increase coverage by 0.38%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master    #3519      +/-   ##
==========================================
+ Coverage   55.47%   55.85%   +0.38%     
==========================================
  Files         105      105              
  Lines       16004    16024      +20     
==========================================
+ Hits         8879     8951      +72     
+ Misses       6376     6324      -52     
  Partials      749      749

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

lgtm

@jackfrancis jackfrancis merged commit 03e6909 into Azure:master Jul 23, 2018
@cpuguy83 cpuguy83 deleted the detect_subid branch July 23, 2018 18:58
juan-lee pushed a commit that referenced this pull request Aug 1, 2018
juan-lee pushed a commit that referenced this pull request Aug 1, 2018
jackfrancis pushed a commit to jackfrancis/acs-engine that referenced this pull request Aug 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants