Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: init Azure clients consistently #2739

Merged
merged 2 commits into from
Feb 19, 2020

Conversation

mboersma
Copy link
Member

Reason for Change:
While adding new Azure client support to our AzureClient, I noticed that not all client objects are being initialized the same way. This appears to be an oversight, so I fixed it.

Issue Fixed:

Requirements:

Notes:

@acs-bot acs-bot added the size/M label Feb 18, 2020
@codecov
Copy link

codecov bot commented Feb 18, 2020

Codecov Report

Merging #2739 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2739      +/-   ##
==========================================
- Coverage   72.33%   72.33%   -0.01%     
==========================================
  Files         137      137              
  Lines       25317    25332      +15     
==========================================
+ Hits        18314    18324      +10     
- Misses       5947     5952       +5     
  Partials     1056     1056              

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1425cd7...1ff020c. Read the comment docs.

@jackfrancis
Copy link
Member

What exactly change? (diff is a bit obtuse here)

@mboersma
Copy link
Member Author

Sorry, the diff is hard to parse. I alpha-sorted each section so I could do a visual comparison and see which were missing. (I would have made this a better refactor but there aren't any existing interfaces to use here and it looks awkward to create one.)

In summary:

  • c.availabilitySetsClient and c.virtualMachineExtensionsClient weren't getting authorizer set as other clients did
  • c.msiClient, c.ServicePrincipalsClient, c.VirtualMachineExtensionsClient, c.VirtualMachineImagesClient, and c.workspacesClient weren't getting PollingDuration set as other clients did
  • Several clients weren't getting RequestInspector set in AddAcceptLanguages
  • Several clients weren't getting RequestInspector set in AddAuxiliaryTokens

With the exception of those clients using graphAuthorizer, it looks like all these init patterns should be identical for the set of clients, and this PR makes that so.

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

@acs-bot acs-bot added the lgtm label Feb 19, 2020
@jackfrancis jackfrancis merged commit b0bd385 into Azure:master Feb 19, 2020
@mboersma mboersma deleted the azcli-consistent-init branch February 19, 2020 00:27
@acs-bot
Copy link

acs-bot commented Feb 19, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma

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:
  • OWNERS [jackfrancis,mboersma]

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants