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

Allow Curl redirect on get #331

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Mar 3, 2023

🎉 New feature

Summary

Retargeting #330 to Citadel. See #330 for more information about redirects.

I'm scoping this change to only GET requests.

Test it

Try running the following command. The Staging Fuel server is setup to redirect requests to S3.

ign fuel download -u https://staging-fuel.gazebosim.org/1.0/marcoshuck5/models/testcoro

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@nkoenig nkoenig requested a review from marcoshuck March 3, 2023 16:52
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Mar 3, 2023
Copy link

@marcoshuck marcoshuck left a 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, we can merge this once we can make sure it works for both private and public models/worlds.

src/RestClient.cc Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #331 (8f36f9e) into ign-fuel-tools4 (91d0b6f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 8f36f9e differs from pull request most recent head dc8773f. Consider uploading reports for the commit dc8773f to get more accurate results

@@                 Coverage Diff                 @@
##           ign-fuel-tools4     #331      +/-   ##
===================================================
+ Coverage            77.64%   77.66%   +0.01%     
===================================================
  Files                   19       19              
  Lines                 2738     2740       +2     
===================================================
+ Hits                  2126     2128       +2     
  Misses                 612      612              
Impacted Files Coverage Δ
src/RestClient.cc 68.38% <100.00%> (+0.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@nkoenig nkoenig merged commit eb6c357 into ign-fuel-tools4 Mar 14, 2023
@nkoenig nkoenig deleted the nkoenig/curl_redirect_on_get branch March 14, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants