-
Couldn't load subscription status.
- Fork 99
Drop Resty as a dependency #576
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
Drop Resty as a dependency #576
Conversation
52823e7 to
92c4da8
Compare
bc4dc02 to
519fdcd
Compare
27bad2e to
117b42b
Compare
2fa956d to
b584283
Compare
c813c02 to
8cbafb0
Compare
39f6fed to
daa932b
Compare
daa932b to
77adeb4
Compare
* build(deps): bump golang.org/x/text from 0.18.0 to 0.19.0 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.18.0 to 0.19.0. - [Release notes](https://github.com/golang/text/releases) - [Commits](golang/text@v0.18.0...v0.19.0) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Ran make tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: ezilber-akamai <ezilber@akamai.com>
* allow some vars to be shared * add target branch * remove target branch * revert to push_request and only run slack notify when push to main
d617c7e to
09a3567
Compare
|
I think it was implemented in a previous PR, but would it make sense for |
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.
I tested this pretty thoroughly and everything seems to be working well, great work!
1b2d5e0 to
c47ec39
Compare
c47ec39 to
6a71272
Compare
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.
LGTM overall, tested locally and also ran tests for terraform. We probably need a follow up ticket to ensure everything runs smooth in terraform and update retry functions. Excellent work!!
| runs-on: ubuntu-latest | ||
| needs: [test] | ||
| if: always() && github.repository == 'linode/linodego' # Run even if integration tests fail and only on main repository | ||
| if: always() && github.ref == 'refs/heads/main' && github.event_name == 'push' # Run even if integration tests fail and only on main repository |
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.
Do we want to add the condition github.ref == 'refs/heads/main' && github.event_name == 'push' along with github.repository == 'linode/linodego' to limit slack notifications?
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.
I think it's already in main? Maybe we can do some main -> proj -> this-branch merges?
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.
Looks good to me! Thanks for addressing these
📝 Description
Linodego currently uses the go-resty package to manage all HTTP requests made to the API. This includes logic for authentication, JSON marshaling/unmarshaling, request retries, and debug outputs.
Unfortunately this package has caused various issues for linodego users, including:
Because of these issues, Resty is being dropped as a dependency in favor of the mature and widely adopted net/http package.
✔️ How to Test
Run the unit test suite with
make testunitRun the integration test suite with
make testint