mohanboddu opened a new pull-request against the project: `releng` that you are following: `` Mass rebuild modules ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: `` WIP, dont merge it yet. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
ignatenkobrain commented on the pull-request: `Mass rebuild modules` that you are following: `` I don't really like `branch_active`, because that wouldn't include libgit2 which got EOL on december which I had to adjust manually, sigh. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: `` @ignatenkobrain Well, there is no other way to figure out which modules are active and which needs rebuilding. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
ignatenkobrain commented on the pull-request: `Mass rebuild modules` that you are following: `` @mohanboddu, what if we take all repos which are shipped now in rawhide? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
@mohanboddu, what if we take all repos which are shipped now in rawhide?
@ignatenkobrain Do you know a way to do this? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Given that this token has privilege to modify any package in the distro, it would probably be best if this was loaded from a file on disk rather than put into the process table where any other user on the system can read it. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` This could use a comment explaining what you're querying for. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` My python-fu is weak here, but can you actually do this? Does it actually check that the dictionaries are equivalent or just that they are the same dictionary? I'd probably be overcautious and make this a dictionary keyed by "module_name:module_stream" instead. Then you can just iterate through the values later. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` This needs to be cloning "modules/module_name". Is that what you get from `sla['branch']['global_component']`? If so, that would be useful to add as a comment around line 79. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` If `name` == `modules/modulename`, then this will always fail, since the clone above is going to make only the `modulename` directory. If `name` == `modulename`, then the clone will be checking out `rpms/modulename` (if it exists). Either way, one of these lines is wrong. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` You need to `chdir()` into the clone directory before running `switch-branch`. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
Given that this token has privilege to modify any package in the distro, it would probably be best if this was loaded from a file on disk rather than put into the process table where any other user on the system can read it.
I would like to run this script normally user rather than a privileged user, thats why I went with providing the token rather than reading it from somewhere. Three issues I thought of were
1. If its stored somewhere only a privileged user can access, then sudo might time out and creating a ton of issues. 2. Lets say if its stored in my home dir somewhere, then no one else can run this script. 3. Some changes are needed to the script like where the dist-git checkout happens and stuff.
``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Don't use `file.endswith()`. It needs to be an exact match. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: ``
Given that this token has privilege to modify any package in the distro, it would probably be best if this was loaded from a file on disk rather than put into the process table where any other user on the system can read it.
I would like to run this script normally user rather than a privileged user, thats why I went with providing the token rather than reading it from somewhere. Three issues I thought of were
If its stored somewhere only a privileged user can access, then sudo might time out and creating a ton of issues. Lets say if its stored in my home dir somewhere, then no one else can run this script. Some changes are needed to the script like where the dist-git checkout happens and stuff.
I think you misunderstood. I meant that the argument to this script should be a path to a file the user has access to. You read it once into memory and use it for the rest of the process. The only difference is that the token doesn't appear in the process table, which is public.
So you'd call `mass-rebuild-modules --token-file=./relengtoken.txt` instead of ``mass-rebuild-modules --token=abcdef1234567890` ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Maybe change this comment to "Use libmodulemd to determine if this module stream applies to this platform version". ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Move this `f30` to a command-line argument so this script can be reused in the future without issue. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` `fedpkg` has no option to allow an empty commit. You'll need to use `git commit --allow-empty` here. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
So you'd call mass-rebuild-modules --token-file=./relengtoken.txt instead of `mass-rebuild-modules --token=abcdef1234567890
Yeah, this is possible, I will adjust it ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
fedpkg has no option to allow an empty commit. You'll need to use git commit --allow-empty here.
Sorry for the wrong comment, its not an empty commit, it will commit saying something like
"Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild" ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: ``
Sorry for the wrong comment, its not an empty commit, it will commit saying something like "Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild"
The commit message will exist but if you have not changed any content in the repo itself, `fedpkg` will exit with nonzero and not make the commit. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: `` FYI, I will squash the commits before merging them ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` This is still trying to compare dictionaries in a list. I don't think that works. Maybe just skip this "if" statement, since it's unlikely you'd ever see a duplicate anyway. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
This is still trying to compare dictionaries in a list. I don't think that works. Maybe just skip this "if" statement, since it's unlikely you'd ever see a duplicate anyway.
I remember getting duplicates while I test ran it, let me try it again and see ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
This is still trying to compare dictionaries in a list. I don't think that works. Maybe just skip this "if" statement, since it's unlikely you'd ever see a duplicate anyway.
There will be duplicates because of sla types like 'security_fixes' or 'bug_fixes' ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Am I missing something? It's completely expected that a module can have more than one stream associated with it. Or am I misreading this? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
Am I missing something? It's completely expected that a module can have more than one stream associated with it. Or am I misreading this?
Each `module` dict has key as its name and stream as its value, so there will be different `module` dicts for different name and stream pairs. Actually this check is not really required, I just added just in case if there are more than 1 key:value pairs. The execution should never hit this. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Shouldn't this be under f30? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Please use `#!/usr/bin/python2`. Also, this file is not marked executable. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` There's an extra trailing close-paren that makes this file fail to parse. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Should this be len > 1, based on the printf after it? This section definitely needs a comment explaining it. It's cluttered and unclear what it's trying to do. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` This will cause the entire run to bail out if the request happened to time out. Is that really what we want? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Wait... are we only doing this section so as to print the "Skipping" notice? That seems wasteful. What's the value? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Please clearly document what this command is supposed to do. I think it gets a list of all module builds of this name and stream submitted after `module_epoch`, but I'm not sure that's going to get you the list of things you still need to build. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Uh, this isn't the stream of the module. When you assign to `module[name]`, you're assigning it the `sla['branch']['name']` but this is always "f29", "master", etc. Not the stream branch of the module. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Drop this `else`. The if statement calls `continue`, so it's unnecessary and makes it harder to follow the flow. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` As noted above, `stream` is wrong here, so you won't be getting the right stream branch. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` Why not just do the `os.path.join()` here and then call `os.path.exists()` like the `noautobuild` check? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
sgallagh commented on the pull-request: `Mass rebuild modules` that you are following: `` You may want to handle the possibility of an exception if you can't read the file, but that should really never happen. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
Wait... are we only doing this section so as to print the "Skipping" notice? That seems wasteful. What's the value?
This is extremely useful when we have to rerun the script because it failed due to unknown reasons (like network glitch or outage) and dont need to rebuild all the modules that are already built. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu commented on the pull-request: `Mass rebuild modules` that you are following: ``
Uh, this isn't the stream of the module. When you assign to module[name], you're assigning it the sla['branch']['name'] but this is always "f29", "master", etc. Not the stream branch of the module.
Stream is the name as per pdc api, for ex nodejs module stream 8 is stored as https://pdc.fedoraproject.org/rest_api/v1/component-branch-slas/?page_size=1... ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8082
mohanboddu merged a pull-request against the project: `releng` that you are following.
Merged pull-request:
`` Mass rebuild modules ``
rel-eng@lists.fedoraproject.org
-
Igor Gnatenko -
Mohan Boddu
-
Stephen Gallagher