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

Add new @azure/app-configuration library #4753

Merged
merged 2 commits into from
Aug 16, 2019
Merged

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented Aug 13, 2019

This PR adds a new @azure/app-configuration library that is a client for the Azure App Configuration service. It is partially generated from an unpublished Swagger spec.

@ramya-rao-a: I've added a starting point for the convenience layer in src/index.ts, all of the nicer API methods should go there.

@daviwil daviwil requested a review from ramya-rao-a as a code owner August 13, 2019 00:14

### Currently supported environments

- Node.js version 6.x.x or higher
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the README configurable at all through autorest? I think we're now saying we only support version 8.x.x or higher, and should be using @azure/identity over @azure/ms-rest-nodeauth if possible.

Since this package also includes the convenience layer we should probably make these changes (though they can probably be done in a separate PR to get this base out), but will it get replaced if we need to regenerate from the models again?

Copy link
Contributor

Choose a reason for hiding this comment

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

When generating code, there is an option to skip the readme

"prepack": "npm install && npm run build"
},
"sideEffects": false,
"autoPublish": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want auto publishing? I believe this means when a PR with a version change is merged, it gets auto published

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, will remove it.


const ConnectionStringRegex = /Endpoint=(.*);Id=(.*);Secret=(.*)/;
const deserializationContentTypes = {
json: ["application/vnd.microsoft.appconfig.kvset+json"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I was testing the operations and found there are actually more content types than this we need to add!
Source: https://github.com/Azure/AppConfiguration/blob/master/docs/REST/kv.md

  • application/vnd.microsoft.appconfig.kv+json
  • application/vnd.microsoft.appconfig.kvs+json
  • application/problem+json

Source: https://github.com/Azure/AppConfiguration/blob/master/docs/REST/keys.md

  • application/vnd.microsoft.appconfig.keyset+json

Source: https://github.com/Azure/AppConfiguration/blob/master/docs/REST/revisions.md

  • application/vnd.microsoft.appconfig.revs+json

Copy link
Contributor Author

@daviwil daviwil Aug 16, 2019

Choose a reason for hiding this comment

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

Oh joy! Glad you caught those. In the future we may need to consider enabling regexes or wildcards in deserializationContentTypes so that we can say something like /application/.*+json/.

@daviwil daviwil changed the title WIP: Add new @azure/app-configuration library Add new @azure/app-configuration library Aug 16, 2019
@daviwil daviwil merged commit a77e544 into Azure:master Aug 16, 2019
@daviwil daviwil deleted the add-app-config branch August 16, 2019 19:57
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.

3 participants