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.

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

dnfjson: allow (optional) field solver in the dnfjson output #723

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jun 3, 2024

With the new "dnf5" solver is seems useful to allow the dnfjson code return what solver was actually used in the transaction.

This commit allows an (optional) "solver" field in the json that is returned from the dnfjson helper. It does not do more yet but we should (probably) make it avaialble for the higher layers too so that it can be e.g. logged.

See also osbuild/osbuild#1776

@mvo5 mvo5 requested a review from bcl June 3, 2024 09:26
@bcl
Copy link
Contributor

bcl commented Jun 3, 2024

Looks good, other than the linting complaints :)

@mvo5 mvo5 force-pushed the depsolve-result-add-solver branch from 4835c79 to e95f344 Compare June 4, 2024 06:58
@mvo5 mvo5 marked this pull request as ready for review June 4, 2024 06:58
@mvo5 mvo5 requested a review from achilleas-k June 4, 2024 07:16
pkg/dnfjson/dnfjson.go Outdated Show resolved Hide resolved
achilleas-k
achilleas-k previously approved these changes Jun 4, 2024
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

LGTM. Technically we can merge this before the osbuild PR and it wouldn't be a problem, but let's do it in proper order.

Then we could also run the unit tests in dnfjson_test.go against both solvers.

bcl
bcl previously approved these changes Jun 4, 2024
Copy link
Contributor

@bcl bcl left a comment

Choose a reason for hiding this comment

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

Looks good, pylint is apparently befuddled by having a raise in an else before using a variable.

@bcl
Copy link
Contributor

bcl commented Jun 4, 2024

Made #729 to ignore the pylint error

Copy link

github-actions bot commented Jul 5, 2024

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 5, 2024
@achilleas-k achilleas-k removed the Stale label Jul 5, 2024
With the new "dnf5" solver is seems useful to allow the dnfjson
code return what solver was actually used in the transaction.

This commit allows an (optional) "solver" field in the json
that is returned from the dnfjson helper. It does not do more
yet but we should (probably) make it avaialble for the higher
layers too so that it can be e.g. logged.

See also osbuild/osbuild#1776

Co-authored-by: Achilleas Koutsou <[email protected]>
@mvo5 mvo5 dismissed stale reviews from bcl and achilleas-k via a8f9e7d August 1, 2024 07:03
@mvo5 mvo5 force-pushed the depsolve-result-add-solver branch from bbe94b8 to a8f9e7d Compare August 1, 2024 07:03
@mvo5 mvo5 requested a review from achilleas-k August 1, 2024 07:03
mvo5 added a commit to mvo5/osbuild that referenced this pull request Aug 1, 2024
This commit includes the used sovler in the dnfjson reply. This
is mostly information (e.g. in service logs) but also useful in
tests to ensure that the expected solver was really run.

Note that this needs osbuild/images#723
first.
mvo5 added a commit to mvo5/osbuild that referenced this pull request Aug 1, 2024
This commit includes the used sovler in the dnfjson reply. This
is mostly information (e.g. in service logs) but also useful in
tests to ensure that the expected solver was really run.

Note that this needs osbuild/images#723
first.
@mvo5 mvo5 added this pull request to the merge queue Aug 1, 2024
Merged via the queue into osbuild:main with commit ae9b3da Aug 1, 2024
18 checks passed
mvo5 added a commit to osbuild/osbuild that referenced this pull request Aug 6, 2024
This commit includes the used sovler in the dnfjson reply. This
is mostly information (e.g. in service logs) but also useful in
tests to ensure that the expected solver was really run.

Note that this needs osbuild/images#723
first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants