DNF5 prototype #5857

pkratoch · 2024-08-28T11:17:25Z

No description provided.

pep8speaks · 2024-08-28T11:17:37Z

Hello @pkratoch! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file pyanaconda/modules/payloads/payload/dnf/transaction_progress.py:

Line 90:13: E265 block comment should start with '# '
Line 119:13: E265 block comment should start with '# '

In the file tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py:

Line 518:100: E501 line too long (100 > 99 characters)

In the file tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py:

Line 26:1: E265 block comment should start with '# '
Line 31:1: E265 block comment should start with '# '
Line 32:1: E265 block comment should start with '# '
Line 33:1: E265 block comment should start with '# '
Line 34:1: E265 block comment should start with '# '
Line 35:1: E265 block comment should start with '# '
Line 36:1: E265 block comment should start with '# '

Comment last updated at 2025-01-06 14:19:49 UTC

pkratoch · 2024-09-13T13:39:02Z

This dnf5 PR is needed for the prototype to finish the installation: rpm-software-management/dnf5#1697

pkratoch · 2024-09-17T09:51:19Z

pyanaconda/modules/payloads/payload/dnf/dnf_manager.py

+class DNFConfigWrapper(object):
+    """This is a temporary wrapper of a DNF config object."""
+
+    def __init__(self, config):
+        """Wrap the DNF config object."""
+        self._config = config
+
+    def __getattr__(self, name):
+        """Get the attribute.
+
+        Called when an attribute lookup has not found
+        the attribute in the usual places.
+        """
+        option = getattr(self._config, name)()
+        return option.get_value()
+
+    def __setattr__(self, name, value):
+        """Set the attribute.
+
+        Called when an attribute assignment is attempted.
+        """
+        if name in ["_config"]:
+            return super().__setattr__(name, value)
+
+        option = getattr(self._config, name)()
+        option.set(value)


Regarding the DNFConfigWrapper, right now, it is quite ugly, since the option names are in the form get_<name>_option. I have two possible solutions:

Remove the wrapper and access the options directly. This would look like this:

config.get_cachedir_option().get_value() config.get_cachedir_option().set(DNF_CACHE_DIR)

Keep the wrapper, but use simple <name> form in anaconda and wrap it with "get_" + name + "_option" in the DNFConfigWrapper.

The second solution would mean more readable access to the options, but the disadvantage is that it relies on a specific naming scheme in libdnf5. This could become a problem if we needed something else from the config e.g. to load config from parser (ConfigMain::load_from_parser), but it's not needed now. I prefer the second solution, but I don't know why the DNFConfigWrapper is described as temporary and what was the plan for future.

I think i prefer to remove the wrapper. Or if you keep the wrapper, create access getters and settings for all used attributes.
I am not fond of the 2nd option - where we assume the "get" + name + "_option" naming scheme
As this can break easily
Does it make sense?

I changed the wrapper to contain all the necessary options as properties. It got quite long, so let me know if it's acceptable. Maybe it could be moved to a separate file? Alternatively, I will change it to remove the wrapper entirely.

@pkratoch I like it how it is now. Thanks

pkratoch · 2024-09-19T08:28:24Z

I also pushed 2 commits changing the mocking in unit tests. Sorry I didn't notice it sooner that the tests do not pass. I am not totally sure about the second one that adds the try block, though. It's ok to do it like that in the test, but I am not sure if we can rely on dnf always sending the quit or error callback. If dnf crashed and didn't send anything, anaconda would wait forever. But I don't think there is anything that can be done on anaconda side.

Also, I put the commits on top of the branch, but I would like to eventually amend the relevant commits in history, so that the history is not so chaotic. Let me know if it's ok to do that.

KKoukiou · 2024-09-19T09:10:10Z

I also pushed 2 commits changing the mocking in unit tests. Sorry I didn't notice it sooner that the tests do not pass. I am not totally sure about the second one that adds the try block, though. It's ok to do it like that in the test, but I am not sure if we can rely on dnf always sending the quit or error callback. If dnf crashed and didn't send anything, anaconda would wait forever. But I don't think there is anything that can be done on anaconda side.

Also, I put the commits on top of the branch, but I would like to eventually amend the relevant commits in history, so that the history is not so chaotic. Let me know if it's ok to do that.

Sure feel free to amend.

jan-kolarik · 2024-11-07T09:15:06Z

pyanaconda/modules/payloads/payload/dnf/dnf_manager.py

+
+    @property
+    def baseurl(self):
+        return self._config.get_baseurl_option().get_value()


This seems not necessary as dnf5 is already providing these attributes in Python bindings. Check the implementation in our SWIG interface file and e.g. this test.

Oh, wow, how did I miss that? It was even done in the same PR as the renaming, but I guess I focused too much on how the wrapped had been done before that I didn't notice. This is really neat, I will change the PR to drop the wrapper and use the attributes directly.

I think this is the case, when presenting a simple demo of new features added to our projects during the sprint review or retrospective would be useful. 🙂

There was a change regarding creation of the `ValidationReport`. It's no longer created in `CheckPackagesSelectionTask._resolve_selection()` but in `DNFManager.resolve_selection()`. The change lookgs good and it was probably even necessary because of changes in dnf reports from the transaction resolving, but it prevents testing the report messages, because the report is now also mocked. Since it's not a critical problem, I am disabling the message checks for now. The changes that caused the issue: poncovka@faccc47#diff-08489d103d5780dd5279cb390820b5eeb83963642783cd82773cef081803b1b7L620-L641 poncovka@faccc47#diff-57dffd56427be4eca900ee75e0a130414fb51527d206b00116ef9501f0784127L96-L114

The comps objects are owned by the queries, so when a query is destroyed, so are the contained objects. There is a bug for dnf: rpm-software-management/dnf5#1530 This is a temporary workaround.

The ValidationReport is now created within the mocked DNFManager.resolve_selection, so it must be mocked as well. Also, there are no longer exceptions raised from the DNF transaction, instead, the problems are stored in a report, so there are no exceptions to mock.

The transaction callbacks take libdnf5::base::TransactionPackage, not libdnf5::transaction::Package.

The base is no longer being closed and the resetting of base is tested in test_module_payload_dnf5_manager.DNFManagerTestCase.test_reset_base.

These tests probably didn't work at the time they were marked as skipped, but are working now.

The transaction can fail even if it doesn't contain any transaction items with an error status.

The transaction_stop is actually for RPMCALLBACK_TRANS_STOP from rpm, which is only the end of preparation phase. The after_complete is called by dnf after the whole transaction completes. Also, the queue cannot be closed at this point, because transaction errors are written there after the transaction completes and then TransactionProgress.quit is called, which closes the queue as well.

In libdnf5, the `repos_updated_and_loaded` bool attribute, which serves the same purpose, is private. Check if the repositories are loaded before making queries.

To reset the repo sack, the whole base must be created anew, because it is not possible to lead repositories multiple times in dnf5.

There is no way to remove a repository from the RepoSack without dropping the entire Base. Therefore, a repository can be only configured again, but not replaced. Since it cannot be removed, the test for repository removal was removed as well.

Progress is reported as summary of how much is downloaded out of total. This also removes the debug logs for all the progress messages and adds only logs for starting and finishing the downloads. The debug logs cannot be in the _report_progress class, because that one is paced.

The exceptions are not actually raised anywhere and the report is now constructed right away in the resolve_selection method, so it doesn't make sense to raise these and catch elsewhere to add the messages to the report as it was done previously.

pkratoch had a problem deploying to gh-cockpituous August 28, 2024 11:17 — with GitHub Actions Failure

github-actions bot added the f42 Fedora 42 label Aug 28, 2024

pkratoch had a problem deploying to gh-cockpituous August 28, 2024 12:29 — with GitHub Actions Failure

pkratoch commented Sep 17, 2024

View reviewed changes

KKoukiou force-pushed the master-dnf5_prototype branch from f14b803 to 83ae896 Compare September 17, 2024 10:52

KKoukiou had a problem deploying to gh-cockpituous September 17, 2024 10:52 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from 83ae896 to ed6f3f2 Compare September 19, 2024 08:24

pkratoch had a problem deploying to gh-cockpituous September 19, 2024 08:25 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from ed6f3f2 to 0365101 Compare September 19, 2024 09:24

pkratoch had a problem deploying to gh-cockpituous September 19, 2024 09:24 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from 0365101 to 34f137e Compare September 19, 2024 09:26

pkratoch had a problem deploying to gh-cockpituous September 19, 2024 09:26 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from 34f137e to fca17eb Compare September 24, 2024 13:11

pkratoch had a problem deploying to gh-cockpituous September 24, 2024 13:11 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from fca17eb to c5b29fc Compare September 26, 2024 11:31

pkratoch had a problem deploying to gh-cockpituous September 26, 2024 11:31 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from c5b29fc to d34a250 Compare September 30, 2024 11:52

pkratoch had a problem deploying to gh-cockpituous September 30, 2024 11:52 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from d34a250 to 4c78c94 Compare October 29, 2024 08:09

pkratoch had a problem deploying to gh-cockpituous October 29, 2024 08:09 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from 4c78c94 to b2d9c31 Compare November 6, 2024 14:45

pkratoch had a problem deploying to gh-cockpituous November 6, 2024 14:45 — with GitHub Actions Failure

jan-kolarik reviewed Nov 7, 2024

View reviewed changes

pkratoch force-pushed the master-dnf5_prototype branch from b2d9c31 to a428b7f Compare November 12, 2024 14:22

pkratoch had a problem deploying to gh-cockpituous November 12, 2024 14:22 — with GitHub Actions Failure

pkratoch force-pushed the master-dnf5_prototype branch from a428b7f to 42e2d38 Compare November 25, 2024 08:06

pkratoch added 28 commits January 6, 2025 15:19

Fix use of group and environment iterators

32c0e93

Update TransactionItemState_ERROR import to libdnf5

2de138a

Remove unused _create_group method from DNFRequirementsTestCase

e7e7734

TEMPORARY: Store comps queries

a007943

The comps objects are owned by the queries, so when a query is destroyed, so are the contained objects. There is a bug for dnf: rpm-software-management/dnf5#1530 This is a temporary workaround.

Fix setting of libdnf5.repo.PackageDownloader

44e5a8a

Enable comps tests

863606b

Adapt checking transaction errors to DNF5

ca9e024

Fix setting up and running transaction

7604541

Adapt tests of DNFManager.install_packages for dnf5

e984502

Fix transaction callbacks

b282fcc

The transaction callbacks take libdnf5::base::TransactionPackage, not libdnf5::transaction::Package.

Report warnings in DNFManager.resolve_selection

0c6e2d0

Remove the test_dnf_tear_down

0ed56aa

The base is no longer being closed and the resetting of base is tested in test_module_payload_dnf5_manager.DNFManagerTestCase.test_reset_base.

Enable some tests in test_module_payload_dnf5_manager

7589a94

These tests probably didn't work at the time they were marked as skipped, but are working now.

Log also the message of the dnf transaction result

7850e8e

Report error when the dnf transaction fails

56104b5

The transaction can fail even if it doesn't contain any transaction items with an error status.

Log messages from dnf into /tmp/dnf.log

c4e24e3

Always call progress.quit in mocked installing packages in tests

fd6dba1

Set the destdir option

67a9850

Remember if repositories were loaded

c0c7410

In libdnf5, the `repos_updated_and_loaded` bool attribute, which serves the same purpose, is private. Check if the repositories are loaded before making queries.

Reset the dnf base when clearing cache

6da04d5

To reset the repo sack, the whole base must be created anew, because it is not possible to lead repositories multiple times in dnf5.

Implement replacing repositories

f89d9b8

There is no way to remove a repository from the RepoSack without dropping the entire Base. Therefore, a repository can be only configured again, but not replaced. Since it cannot be removed, the test for repository removal was removed as well.

Respect the missing_ignored and broken_ignored configuration

7b847a8

Remove unused imports

320ccfc

pkratoch force-pushed the master-dnf5_prototype branch from f7d5841 to 48132b1 Compare January 6, 2025 14:19

pkratoch had a problem deploying to gh-cockpituous January 6, 2025 14:19 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DNF5 prototype #5857

DNF5 prototype #5857

pkratoch commented Aug 28, 2024

pep8speaks commented Aug 28, 2024 •

edited

Loading

pkratoch commented Sep 13, 2024

pkratoch Sep 17, 2024

KKoukiou Sep 17, 2024

pkratoch Sep 19, 2024

KKoukiou Sep 20, 2024

pkratoch commented Sep 19, 2024

KKoukiou commented Sep 19, 2024

jan-kolarik Nov 7, 2024

pkratoch Nov 7, 2024

jan-kolarik Nov 7, 2024

DNF5 prototype #5857

Are you sure you want to change the base?

DNF5 prototype #5857

Conversation

pkratoch commented Aug 28, 2024

pep8speaks commented Aug 28, 2024 • edited Loading

Comment last updated at 2025-01-06 14:19:49 UTC

pkratoch commented Sep 13, 2024

pkratoch Sep 17, 2024

Choose a reason for hiding this comment

KKoukiou Sep 17, 2024

Choose a reason for hiding this comment

pkratoch Sep 19, 2024

Choose a reason for hiding this comment

KKoukiou Sep 20, 2024

Choose a reason for hiding this comment

pkratoch commented Sep 19, 2024

KKoukiou commented Sep 19, 2024

jan-kolarik Nov 7, 2024

Choose a reason for hiding this comment

pkratoch Nov 7, 2024

Choose a reason for hiding this comment

jan-kolarik Nov 7, 2024

Choose a reason for hiding this comment

pep8speaks commented Aug 28, 2024 •

edited

Loading