feat: Add JSON output for updateinfo #2200
Conversation
|
Hello @walkerever! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2025-01-28 01:22:39 UTC |
walkerever
force-pushed
the
json-output-updateinfo
branch
10 times, most recently
from
2325ee1 to
8fead9a
Compare
|
for A test about |
walkerever
force-pushed
the
json-output-updateinfo
branch
from
8fead9a to
036c511
Compare
|
Known limit as best effort without introducing significant code changes:
|
walkerever
force-pushed
the
json-output-updateinfo
branch
from
036c511 to
6119ab0
Compare
| @@ -157,6 +162,10 @@ def configure(self): | |||
| else: | |||
| self.opts.spec.insert(0, spec) | |||
|
|
|||
| # Keep quiet when dumping JSON output | |||
| if self.opts.json: | |||
| self.cli.redirect_logger(stdout=sys.maxsize, stderr=sys.maxsize) | |||
There was a problem hiding this comment.
This is good but there still are some redundant messages in the output, namely I think the download progress bars.
We could additionally do something similar to what the --quiet option does: https://github.com/rpm-software-management/dnf/blob/master/dnf/cli/cli.py#L834-L836
To take effect I believe it would have to be set in pre_configure of the command.
I would add something like:
def pre_configure(self):
if self.opts.json:
self.base.conf.debuglevel = 0Perhaps don't change the errorlevel since those message are more crucial.
| dtlst.append( | ||
| { | ||
| "name": aid, | ||
| "type": atype, | ||
| "severity": asev, | ||
| "nevra": nevra, | ||
| "buildtime": aupdated, |
There was a problem hiding this comment.
dnf5 has special logic when --with-bz or --with-cve is used: https://github.com/rpm-software-management/dnf5/blob/main/dnf5/commands/advisory/advisory_list.cpp#L75-L81
It adds references to the list subcommand and changes the IDs I think it would be good to be compatible as much as possible. Could you look into that?
| pkg_str += f".{pkg_info.get('arch')}" | ||
| package_list.append(pkg_str) | ||
|
|
||
| REFERENCE_TYPES = {0: 'unknown', 1: 'bugzilla', 2: 'cve', 3: 'vendor', 4: 'security'} |
There was a problem hiding this comment.
I think there is no reference type security.
Also you should not define the numbers here by hand, it would be better to use the exported hawkey constatns:
REFERENCE_TYPES = {hawkey.REFERENCE_UNKNOWN: 'unknown', hawkey.REFERENCE_BUGZILLA: 'bugzilla',
hawkey.REFERENCE_CVE: 'cve', hawkey.REFERENCE_VENDOR: 'vendor'}| asev = self.SECURITY2LABEL.get(sev, _('None')) | ||
| asev = asev.split("/")[0].strip() |
There was a problem hiding this comment.
I think a simpler solution would be to just do:
asev = sevIt is also what dnf5 does so the output will be more compatible.
There was a problem hiding this comment.
for severity, for dnf updateinfo --list output, the format is Medium/Sec for cve type, and single word for others,
i FEDORA-2024-3c18fe0d93 Important/Sec. python-unversioned-command-3.13.1-2.fc41.noarch
i FEDORA-2025-e911f71d99 Moderate/Sec. python-unversioned-command-3.13.2-1.fc41.noarch
i FEDORA-2024-3c18fe0d93 Important/Sec. python3-3.13.1-2.fc41.x86_64
i FEDORA-2025-e911f71d99 Moderate/Sec. python3-3.13.2-1.fc41.x86_64
i FEDORA-2025-52b16605d4 bugfix python3-argcomplete-3.5.3-1.fc41.noarch
i FEDORA-2025-397632c71b bugfix python3-babel-2.17.0-1.fc41.noarch
while for dnf5 json output, it's simply Medium. example as below. reference rpm-software-management/dnf5#1531. that was the reason asev didn't copy from sev.
# dnf advisory list --json | head -20
[
{
"name":"FEDORA-2024-56efaa7783",
"type":"enhancement",
"severity":"Low",
"nevra":"alternatives-1.31-1.fc41.x86_64",
"buildtime":"2024-12-22 02:00:45"
},
{
"name":"FEDORA-2025-9bef5569b2",
"type":"enhancement",
"severity":"None",
"nevra":"audit-libs-4.0.3-1.fc41.x86_64",
"buildtime":"2025-01-10 01:32:24"
},
{
"name":"FEDORA-2024-4b75866373",
"type":"bugfix",
"severity":"Low",
"nevra":"coreutils-9.5-11.fc41.x86_64",
There was a problem hiding this comment.
For dnf4 the list output contains a type (bugfix, enhancement, security..) and when the type is security it also contains the severity (Low, Moderate, Important...) so it combines them both.
With dnf5 json both type and severity are always present but they are separated. I think this is better for the json output.
| advisories = set() | ||
| for apkg, advisory, installed in apkg_adv_insts: | ||
| advisories.add(advisory2info(advisory, installed)) | ||
| dt_advisories.update(self._process_advisory(advisory)) |
There was a problem hiding this comment.
I would prefer if we did this only when the self.opts.json is set otherwise its running always even when not used.
I think you can make it work, just remove the backslashes but keep the indent: 'Vendor': (getattr(advisory, 'vendor', None)
or getattr(advisory, 'author', None) |
Yes, I agree, I am only thinking if it makes sense to show the field if it will always be empty. |
100%;) let me remove them and resubmit. |
This is a backporting for the feature introduced to dnf5 by the following pull requests: - rpm-software-management/dnf5#1531 - rpm-software-management/dnf5#1970 The feature enables JSON format output for updateinfo command.
ack;) uh, |
walkerever
force-pushed
the
json-output-updateinfo
branch
from
6119ab0 to
9b59c70
Compare
removed. tests re-triggered. |
|
oh, spec got updated? |
looks they were removed from I'll go with the code change for now;) |
Yea, though I don't think you need to rebase. If you do need the new libdnf there are builds in https://copr.fedorainfracloud.org/coprs/rpmsoftwaremanagement/dnf-nightly/ just be aware there is also dnf5 in there. |
This is a backporting for the feature introduced to dnf5 by the following pull requests:
The feature enables JSON format output for updateinfo command.