pre_resolve_module_compose.py: Handle change to module separator #921
Conversation
|
Will redo this against current master when I get a chance. |
Instead of building from source, use the Fedora updates-testing packages. Push a couple of fixes into the Dockerfile: containerbuildsystem/osbs-client#730 containerbuildsystem/atomic-reactor#920 containerbuildsystem/atomic-reactor#921
owtaylor
force-pushed
the
module-separator-change
branch
from
7854e9c
to
1328f2e
Compare
atomic_reactor/util.py
Outdated
| m = re.match(r'^(.+)-([^-]+)$', module) | ||
| if not m: | ||
| raise RuntimeError( | ||
| 'Module specification should be NAME:STREAM or NAME:STREAM:VERSION. (NAME-STREAM and NAME-STREAM-VERSION supported for compatibility.)') |
There was a problem hiding this comment.
E501 line too long (148 > 100 characters)
owtaylor
force-pushed
the
module-separator-change
branch
2 times, most recently
from
96cefce
to
1f7a0e5
Compare
|
Rebasing over the change introducing |
|
Looks great! I tried verifying the test changes in the first commit by undoing the change to the plugin, but it is still passing. I'm not sure it's testing it right. |
atomic_reactor/util.py
Outdated
| raise RuntimeError( | ||
| 'Module specification should be NAME:STREAM or NAME:STREAM:VERSION. ' + | ||
| '(NAME-STREAM and NAME-STREAM-VERSION supported for compatibility.)' | ||
| ) |
There was a problem hiding this comment.
This could be rewritten with a list of patterns and a for/break/else iterator. I don't think it would be much shorter, but it would be easier to extend in the future.
The repeated if not m statements aren't wrong, but something about them bothers me.
When I revert the change to the plugin and just have the test changes from the first commit, I get 14/20 fails for 'pytest-3 tests/plugins/test_resolve_module_compose.py. Not sure what I'm doing differently. |
The 'cleanup flatpaks' commit that switched to reading modules from
container.yaml contained a typo:
if not self.data or not self.compose_ids:
raise RuntimeError('"compose" config not set and compose_ids not given')
(Note "or" vs. "and") which meant that large sections of the tests didn't
work as expected. Fix the handling of module/compose IDs to make sense:
container.yaml needs to be present and contain at least one module
in the compose section whether or not compose_ids is set.
Remove a couple of unneeded tests from the parameterization.
Module versioning was changed from NAME-VERSION-RELEASE to a new format with colons - NAME:VERSION:RELEASE (and other variants.) We could get either format from the compose server when resolving modules, depending on how recent a version it is, so accept either format.
owtaylor
force-pushed
the
module-separator-change
branch
from
1f7a0e5
to
2e203da
Compare
|
Pushed a new version that improves a commit message, fixes an error message ('s required Flatpak' => is required for Flapaks), and changes the construct that @mlangsdorf didn't like. I don't have a strong opinion as long as there's not indefinite nesting of ifs. |
|
Oh, the tests were being skipped because I didn't have modulemd or pdc_client installed. Sorry for the noise. |
|
Draft release notes updated. |
It was officially decided that module versions are formatted as N:V:R not N-V-R.
ODCS on input accepts either, so keep on passing N-V-R, but we need to accept
N:V:R if we receive it.