-
Notifications
You must be signed in to change notification settings - Fork 20
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
do-release-tags: Add remote pull option #6
base: master
Are you sure you want to change the base?
Conversation
Allows users to automate the pull and promotion of content from external repositories. This adds two new optional YAML key value pair 'remote' and 'sourceref'. The remote YAML key can also be specified as a command line argument (--remote) sourceref is the refernce in the remote repositoy to pull. Because we are explicitly specifying the commit to pull it doesn't actually doesn't matter what value you use. You will probably want to set it to the name of the reference used upstream because it will show up when you run `ostree refs` on your destination repository. If sourceref isn't specified it defaults to baseref/release.
Not sure if anyone wants to be adding additional YAML key value pairs. This could be implemented in command line arguments which would be more visible to the end user. Otherwise currently the readme links to the CentOS release.yml for documentation. Since that document doesn't include the new options I would guess that I should probably include some documentation or at least an example file? Is there any way that you would prefer to handle that? |
@@ -67,7 +93,21 @@ for (name,newcommit) in releasedata['releases'].iteritems(): | |||
print("No changes in {} = {}, promoted from {}".format(ref, current, currentpromoted)) | |||
continue | |||
print("Previously: {} = {}, promoted from {}".format(ref, current, currentpromoted)) | |||
_,newcommitdata,_ = r.load_commit(newcommit) | |||
try: | |||
_,newcommitdata,_ = r.load_commit(newcommit) |
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'd make this
if havecommit:
to avoid catching things like I/O errors. But not a big deal.
parser = argparse.ArgumentParser(prog=sys.argv[0]) | ||
parser.add_argument("--ignore-no-previous-promotion", help="Releases file", | ||
action='store_true') | ||
parser.add_argument("--repo", help="Repo path", | ||
action='store', required=True) | ||
parser.add_argument("--releases", help="Releases file", | ||
action='store', required=True) | ||
parser.add_argument("--remote", help="Releases file", |
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.
help="Remote name"
Thanks for the PR! Hmm, interesting proposal. I'm a little bit uncertain about the value of having this in do-release-tags though. It feels like something that could just as easily live as a one-line shell invocation of Basically if we were going farther down this route I'd say what we should be building is a library (I guess Python) for things. That said...I'm not opposed to this. If you find it useful today, it's not a lot of code to carry along. But to repeat the previous line, if we're doing more things like this we should have a clear architecture plan. |
I thought the same thing about having the build process just run an ostree command. Having the build process handle the pull was the first thing that I attempted for my own process, but quickly things became more complicated. As far as extending the script further than may be necessary I thought the same thing, so I was a little concerned about you accepting the pull request. The main issue was that I don't think in most cases the build process itself is going to get the new commit IDs as input (environment variables/arguments), So somehow the build script it is going to need to read the release YAML file. Reading a YAML file from bash is possible but it would either require nonstandard utilities or some overly complicated scripts. It seemed like reading the YAML file is something that we should do with a higher language that has a library for such things. Since I had examples with this script I went down the Python path. Pulling examples from do-release-tags, I got the pull_with_options function to work but I realized that I had reinvented the wheel. do-release-tags already opens a repo, reads a YAML file, and iterates over a set of commit ID+ref names, which was the majority of what I was trying to accomplish. So the options were to copy+gut the majority of do-release-tags and create a new script, or else to just combine the functionality. I would have been fine with a new script, but I figured that would just mean more overhead maintaining both scripts. Thanks! |
Allows users to automate the pull and promotion of content from external repositories. This adds two new optional YAML key value pair 'remote' and 'sourceref'.
The remote YAML key can also be specified as a command line argument (--remote)
sourceref is the refernce in the remote repositoy to pull. Because we are explicitly specifying the commit to pull it doesn't actually doesn't matter what value you use. You will probably want to set it to the name of the reference used upstream because it will show up when you run
ostree refs
on your destination repository. If sourceref isn't specified it defaults to baseref/release.See #5.