Skip to content

Comments

[KeyVault] Updating KeyVaultAuthentication signed_session to support session reuse.#2345

Merged
schaabs merged 3 commits intoAzure:keyvault_1.0_previewfrom
schaabs:signed-session-fix
Apr 4, 2018
Merged

[KeyVault] Updating KeyVaultAuthentication signed_session to support session reuse.#2345
schaabs merged 3 commits intoAzure:keyvault_1.0_previewfrom
schaabs:signed-session-fix

Conversation

@schaabs
Copy link

@schaabs schaabs commented Apr 3, 2018

No description provided.

@schaabs schaabs requested a review from lmazuel April 3, 2018 23:57
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

def signed_session(self):
session = requests.Session()
def signed_session(self, session=None):
session = session or super(KeyVaultAuthentication, self).signed_session()
Copy link
Member

Choose a reason for hiding this comment

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

If you user super here, you need to restore the init call in the init (which I see is commented). Most simple here will be:

session = session or requests.Session()

Copy link
Member

Choose a reason for hiding this comment

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

Or change the base class to simply Authentication, since its init isn't doing anything (not like OAuthTokenAuthentication)

@codecov-io
Copy link

codecov-io commented Apr 4, 2018

Codecov Report

Merging #2345 into keyvault_1.0_preview will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@                  Coverage Diff                  @@
##           keyvault_1.0_preview    #2345   +/-   ##
=====================================================
  Coverage                 54.46%   54.46%           
=====================================================
  Files                      5407     5407           
  Lines                    127479   127479           
=====================================================
  Hits                      69429    69429           
  Misses                    58050    58050
Impacted Files Coverage Δ
.../azure/keyvault/custom/key_vault_authentication.py 45.79% <100%> (ø) ⬆️

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 7c7e1fe...33b3ffd. Read the comment docs.

@schaabs schaabs merged commit 5f4ee66 into Azure:keyvault_1.0_preview Apr 4, 2018
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

Successfully merging this pull request may close these issues.

4 participants