search

chaostoolkit-incubator / chaostoolkit-azure

Chaos Toolkit Extension for Azure
https://chaostoolkit.org/
Apache License 2.0
22 stars 28 forks source link

fix README #68

Closed torumakabe closed 5 years ago

torumakabe commented 5 years ago

Correct usage passing environment variables

Lawouach commented 5 years ago

Hi @ToruMakabe

Thanks for this, I think it makes sense because these values are mostly read from the environment. But, the original README could have been interepreted as "these values are hardcoded in the experiment". Maybe, the change should reflect they can be both, not one over the other?

Mind you, if the most common use case is via environment variable, maybe it makes sense to default our doc to this?

cc @bugra-derre

torumakabe commented 5 years ago

Hi @Lawouach

Is "the original README" you mentioned the following page? https://docs.chaostoolkit.org/drivers/azure/

If so, I think the change should reflect both docs and would be default. Because the way is "recommended" and in terms of security.

Thanks

Lawouach commented 5 years ago

I should have been clearer, by original I meant "before your change". Good news, the page you refer is a copy of the readme in this repo (but only updated on each release of the extension)

torumakabe commented 5 years ago

@Lawouach Thank you for your explanation.

There are two ways of handling secret on this doc, using environment variables and hardcoding. In my understanding, these are independent. My PR is only for the case of using environment variables. The original code does not work, so the purpose of this PR is to fix it.

Could you tell me if I make a misunderstanding? Thanks!

Lawouach commented 5 years ago

It does indeed. Sorry if I confused you.