Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making secrets non-required, adding ref-docs #16087

Merged
merged 11 commits into from
Jul 1, 2021
Merged

Conversation

KarishmaGhiya
Copy link
Member

@KarishmaGhiya KarishmaGhiya commented Jun 29, 2021

Making secrets non-required, adding ref-docs:

  • Making all the secrets non-required and adding ref-docs to assist user for create methods
  • Log Analytics - moved 3 parameters that were only for Basic Auth

Others:

  • Re-enabled Eventhub tests
  • Fixed recordings and param for app insights - replacing query param with a space character isntead of nbsp chars
  • Added Changelog

@chradek
Copy link
Contributor

chradek commented Jun 30, 2021

So, you explained to me offline that the sensitive fields are optional because the same interfaces are used both as input and output to/from the service, and the service will never fill in those fields.

@xirzec had commented on the API view that a better experience would be to keep the sensitive fields as required when sending them to the service, but omit them entirely from the output interface. I do agree that this would be an improvement over making the fields always be optional.

It would make sense for these fields to be optional if they were part of a patch operation, but that doesn't appear to be the case here.

Separately, it looks like some tests may need to be re-recorded for the CI to pass.

@KarishmaGhiya
Copy link
Member Author

@chradek Yes I could either make separate interfaces or add in ref docs for the user. @xirzec and I had synced up off line to discuss this.


### Breaking Changes
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably worth a note that the library is also now GA 😄

@KarishmaGhiya KarishmaGhiya enabled auto-merge (squash) July 1, 2021 07:46
@KarishmaGhiya KarishmaGhiya merged commit 9b75850 into main Jul 1, 2021
@KarishmaGhiya KarishmaGhiya deleted the update-identity-MA branch July 1, 2021 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants