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

[merged] Add URL override for fetching objects #271

Closed

Conversation

krnowak
Copy link
Contributor

@krnowak krnowak commented Apr 20, 2016

This adds an override-url option to ostree_repo_pull_with_options, which, if specified, overrides the URL taken from repo config for a given remote.

This also adds a --url option to ostree pull which uses the above option.

The "override-url" option allows to use the other URL while still
using some options from the passed remote.
This allows ostree to pull the objects from a different URL without
modifying the repo's config.
@rh-atomic-bot
Copy link

Can one of the admins verify this patch?
I understand the following comments:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@cgwalters
Copy link
Member

Can you add a test case? Examples are in tests/test-pull-commit-only.sh and tests/pull-test.sh etc.

@cgwalters
Copy link
Member

One test case would be to have a repo that has an upstream remote, clone the remote into a different path, then rm -rf the original, then do a pull with --url from the mirror.

@krnowak
Copy link
Contributor Author

krnowak commented Apr 21, 2016

Sure, will get to it tomorrow.

Might be useful to see what files the client wants to fetch.
@krnowak
Copy link
Contributor Author

krnowak commented Apr 22, 2016

Updated. I added a --log-file flag to trivial-httpd, because I couldn't get why the test was failing. Apparently I did not pass the --mirror flag to ostree pull, so fetching refs/heads/main from the mirror was returning 404. Not sure if you want it in a separate PR, please let me know.

{
GOutputStream *stream = NULL;

if (g_strcmp0(opt_log, "-") == 0)
Copy link
Member

Choose a reason for hiding this comment

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

Missing space between identifier and paren here.

@cgwalters
Copy link
Member

Test LGTM.

@krnowak
Copy link
Contributor Author

krnowak commented Apr 22, 2016

Updated. Added the check for --daemonize and --log-file=- too.

@cgwalters
Copy link
Member

@cgwalters-bot r+ 27fa1ac

@cgwalters
Copy link
Member

In the future, I generally prefer test cases to come in the same commit as the code introducing them. Not a big deal though.

@krnowak
Copy link
Contributor Author

krnowak commented Apr 22, 2016

Alright, will remember next time.

@cgwalters-bot
Copy link

⌛ Testing commit 27fa1ac with merge c293158...

cgwalters-bot pushed a commit that referenced this pull request Apr 22, 2016
This allows ostree to pull the objects from a different URL without
modifying the repo's config.

Closes: #271
Approved by: cgwalters
cgwalters-bot pushed a commit that referenced this pull request Apr 22, 2016
Might be useful to see what files the client wants to fetch.

Closes: #271
Approved by: cgwalters
cgwalters-bot pushed a commit that referenced this pull request Apr 22, 2016
Closes: #271
Approved by: cgwalters
@cgwalters-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing c293158 to master...

@cgwalters-bot cgwalters-bot changed the title Add URL override for fetching objects [merged] Add URL override for fetching objects Apr 22, 2016
rfairley pushed a commit to rfairley/ostree that referenced this pull request Apr 17, 2019
Split off from ostreedev#270

This changes all the places where we muck with paths to use
`os.path.join()`.  Hopefully this is start of some coding standards as
discussed in ostreedev#271.
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.

None yet

4 participants