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

Remove azure resourceGroupClient from backend initialisation. #158

Merged
merged 1 commit into from
May 28, 2020

Conversation

gtardif
Copy link
Contributor

@gtardif gtardif commented May 28, 2020

What I did
azure resourceGroupClient is now obtained lazily instead of being initialised when creating azure Backend.

Several advantages :

  • less dependencies to init backend. (Will need to work more on this to make login command not need an existing subscription ID)
  • backend does not get login token at init time. For a long running process (grace server) this would cause issues with token lifetime
  • we benefit from the client config (polling options especially) set in aci.go, without duplicating stuff

(not mandatory) A picture of a cute animal, if possible in relation with what you did
image

… advantages :

- less dependencies to init backend. (Will need to work more on this to make login command not need an existing subscription ID)
- backend does not get login token at init time. For a long running process (grace server) this would cause issues with token lifetime
- we benefit from the client config (polling options especially) set in aci.go, without duplicating stuff
Copy link
Contributor

@rumpl rumpl 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 we should maybe have an internal struct that we setup lazily rather than these different clients from simple functions

@gtardif
Copy link
Contributor Author

gtardif commented May 28, 2020

Yes, a common azure client struct would regroup code and avoid separate functions like this, will look into this in follow up PRs

@gtardif gtardif merged commit c8acbc3 into master May 28, 2020
@gtardif gtardif deleted the azure_backend_dependency branch May 28, 2020 08:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants