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.

Sign up for GitHub

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

Jump to bottom

Module multistream conflict #1700

Closed
wants to merge 2 commits into from
Closed

Module multistream conflict #1700

wants to merge 2 commits into from

Conversation

j-mracek
Copy link
Member

@j-mracek j-mracek commented Dec 9, 2020

@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

⚠️ Please note that our current plans include removal of these comments in the near future (at least 2 weeks after including this disclaimer), if you have serious concerns regarding their removal or would like to continue receiving them please reach out to us. ⚠️

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/rpm-software-management-dnf-1700
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

keszybz
keszybz reviewed Dec 9, 2020
View reviewed changes
dnf/module/module_base.py Outdated Show resolved Hide resolved
dnf/module/module_base.py
@@ -415,7 +417,13 @@ def _create_module_dict_and_enable(self, module_list, enable=True):
if len(streamDict) > 1:
if moduleState != STATE_DEFAULT and moduleState != STATE_ENABLED \
and moduleState != STATE_DISABLED:
raise EnableMultipleStreamsException(moduleName)
streams_str = "', '".join(
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, why the nested quotes? Shouldn't stream names be composed of characters which don't require quoting?

Copy link
Member Author

Choose a reason for hiding this comment

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

I use 4 streams ('8', '10', '11', '12') to make it similar with other part of message ('name', 'argument')

@m-blaha m-blaha self-assigned this Dec 9, 2020
@m-blaha
Copy link
Member

m-blaha commented Dec 9, 2020

Please add bug reference (RhBug:1814831) to the commit message subject.

@j-mracek
Copy link
Member Author

j-mracek commented Dec 9, 2020

Thanks for comments

@m-blaha
Copy link
Member

m-blaha commented Dec 10, 2020

Thanks, LGTM!

@m-blaha
Copy link
Member

m-blaha commented Dec 10, 2020

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit d98d1d3 has been approved by m-blaha

@rh-atomic-bot
Copy link

⌛ Testing commit d98d1d3 with merge 5d0ed7c...

rh-atomic-bot pushed a commit that referenced this pull request Dec 10, 2020
@j-mracek @rh-atomic-bot
Make an error message more informative (RhBug:1814831)
7b81ce2 
The new message will contain information about related argument and
matched streams.

https://bugzilla.redhat.com/show_bug.cgi?id=1814831

Closes: #1700
Approved by: m-blaha
rh-atomic-bot pushed a commit that referenced this pull request Dec 10, 2020
@j-mracek @rh-atomic-bot
Set an modular error strings for translation
5d0ed7c 
Closes: #1700
Approved by: m-blaha
@rh-atomic-bot
Copy link

💔 Test failed - status-papr

@j-mracek
Copy link
Member Author

One ot the tests failed ( plugins-core.dnf-3-reposync.Tests for reposync command.Reposync with --downloadcomps option (the comps.xml in repodata is not compressed)) but I think that it is not related with the patch.

@kontura
Copy link
Contributor

kontura commented Dec 10, 2020

It is possible the test failed because I have merged the reposync code just after the CI started running here, so it didn't use the new code in dnf but it did use the new test. Lets try again to make sure.

@rh-atomic-bot retry

@rh-atomic-bot
Copy link

⌛ Testing commit d98d1d3 with merge 8c481f9...

rh-atomic-bot pushed a commit that referenced this pull request Dec 10, 2020
@j-mracek @rh-atomic-bot
Make an error message more informative (RhBug:1814831)
6748342 
The new message will contain information about related argument and
matched streams.

https://bugzilla.redhat.com/show_bug.cgi?id=1814831

Closes: #1700
Approved by: m-blaha
rh-atomic-bot pushed a commit that referenced this pull request Dec 10, 2020
@j-mracek @rh-atomic-bot
Set an modular error strings for translation
8c481f9 
Closes: #1700
Approved by: m-blaha
@rh-atomic-bot
Copy link

💔 Test failed - status-papr

@kontura
Copy link
Contributor

kontura commented Dec 10, 2020

@rh-atomic-bot retry

@rh-atomic-bot
Copy link

⌛ Testing commit d98d1d3 with merge b73d813...

rh-atomic-bot pushed a commit that referenced this pull request Dec 10, 2020
@j-mracek @rh-atomic-bot
Set an modular error strings for translation
b73d813 
Closes: #1700
Approved by: m-blaha
@rh-atomic-bot
Copy link

☀️ Test successful - status-papr
Approved by: m-blaha
Pushing b73d813 to master...

inknos added a commit to inknos/ci-dnf-stack that referenced this pull request Feb 2, 2021
@inknos
Update tag @not.with_os=rhel__eq__8
6a82b08 
Requires:
https://bugzilla.redhat.com/show_bug.cgi?id=1814831
rpm-software-management/dnf#1700
dmach pushed a commit to rpm-software-management/ci-dnf-stack that referenced this pull request Feb 2, 2021
@inknos @dmach
Update tag @not.with_os=rhel__eq__8
ac99525 
Requires:
https://bugzilla.redhat.com/show_bug.cgi?id=1814831
rpm-software-management/dnf#1700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keszybz keszybz keszybz left review comments

Assignees

@m-blaha m-blaha

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@j-mracek @m-blaha @rh-atomic-bot @kontura @keszybz