-
Notifications
You must be signed in to change notification settings - Fork 29
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
fix: quarantine axios config #62
Conversation
Codecov Report
@@ Coverage Diff @@
## master #62 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 49 48 -1
Branches 9 12 +3
=====================================
- Hits 49 48 -1
Continue to review full report at Codecov.
|
@ofrobots @stephenplusplus bumpity bumble bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me, and quarantining axios sounds like the right thing to do. It would be good if @stephenplusplus can take a look too.
README.md
Outdated
const res = await gcpMetadata.instance('hostname'); | ||
console.log(res.data) // ...All metadata properties | ||
const data = await gcpMetadata.instance('hostname'); | ||
console.log(data) // ...All metadata properties |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
pair.invalid}' is not a valid configuration option. Please use '${ | ||
pair.expected}' instead. This library is using Axios for requests. Please see https://github.com/axios/axios to learn more about the valid request options.`; | ||
throw new Error(e); | ||
Object.keys(options).forEach(key => { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
BREAKING CHANGE: The
instance()
andproject()
methods are much more selective about which properties they will accept. The only accepted properties areparams
andproperties
.The
instance()
andproject()
methods also now directly return the data instead of a response object.I am trying to quarantine the types for Axios. This makes it so we don't expose the underlying http client library. I am also making the set of acceptable properties you can pass into the options config an allow-list instead of an open obj, and checking it at runtime. This is potentially very breaking, so I want thoughts here from @stephenplusplus and @ofrobots :)