Skip to content

Implement a debugging reporter for collection dependency resolver - #81693

Open
webknjaz wants to merge 2 commits into
ansible:develfrom
webknjaz:features/galaxy-collection-depresolver-debugging-reporter
Open

Implement a debugging reporter for collection dependency resolver#81693
webknjaz wants to merge 2 commits into
ansible:develfrom
webknjaz:features/galaxy-collection-depresolver-debugging-reporter

Conversation

@webknjaz

@webknjaz webknjaz commented Sep 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Member
SUMMARY

This change adds a dependency resolution reporter for tracing the decisions resolvelib is doing.

ISSUE TYPE
  • Feature Pull Request
ADDITIONAL INFORMATION

@webknjaz webknjaz self-assigned this Sep 13, 2023
@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. labels Sep 13, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Sep 13, 2023
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed ci_verified Changes made in this PR are causing tests to fail. labels Sep 13, 2023
@ansibot

This comment was marked as off-topic.

Sign in to view
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Sep 13, 2023
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Sep 14, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 21, 2023
@ansibot ansibot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Dec 28, 2024
@webknjaz

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@ansibot ansibot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Jan 21, 2025
@webknjaz

This comment was marked as outdated.

Sign in to view
@webknjaz webknjaz added the unimportant_ci This PR does not need to have healthy CI status and should be ignored by the CI infra maintainers. label Jan 21, 2025
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 21, 2025
@webknjaz webknjaz mentioned this pull request Jan 21, 2025
Closed
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 4, 2025
@webknjaz webknjaz moved this from 📑 Ready 👌 to 🗒️ Backlog 📝 in 📅 Procrastinating in public 😵‍💫 Feb 28, 2025
@webknjaz
webknjaz force-pushed the features/galaxy-collection-depresolver-debugging-reporter branch from f9f4590 to ea78360 Compare February 28, 2025 23:25
@ansibot ansibot removed stale_pr This PR has not been pushed to for more than one year. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 28, 2025
@ansibot

This comment was marked as outdated.

Sign in to view
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 10, 2025
@webknjaz
webknjaz force-pushed the features/galaxy-collection-depresolver-debugging-reporter branch from ea78360 to 92502be Compare March 11, 2025 16:00
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 11, 2025
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 11, 2025
@webknjaz
webknjaz marked this pull request as ready for review March 12, 2025 14:59
@webknjaz
webknjaz requested review from jborean93 and s-hertel March 12, 2025 15:00
@webknjaz

webknjaz commented Mar 12, 2025

Copy link
Copy Markdown
Member Author

@jborean93 @s-hertel so I've experimented with the extending the depresolver troubleshooting capabilities. Plz play with this and let me know what you think...

@webknjaz webknjaz removed the unimportant_ci This PR does not need to have healthy CI status and should be ignored by the CI infra maintainers. label Mar 12, 2025
@webknjaz webknjaz moved this from 🗒️ Backlog 📝 to 🫸In review🫷 in 📅 Procrastinating in public 😵‍💫 Mar 12, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 19, 2025
s-hertel
s-hertel previously approved these changes Mar 25, 2025
View reviewed changes

@s-hertel s-hertel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could use a changelog.

Comment thread lib/ansible/galaxy/dependency_resolution/__init__.py
include_signatures=include_signatures,
),
CollectionDependencyReporter(),
CollectionDependencyDebuggingReporter() if C.DEFAULT_DEBUG

@s-hertel s-hertel Mar 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add a dedicated config for the ansible-galaxy reporter mode, so its easy to add new reporters in the future?

@webknjaz webknjaz May 12, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should we still consult the debug mode along with that?

@webknjaz webknjaz Jul 23, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@s-hertel since it's taking this long, I think it's best to skip extending the scope of the PR with possibly iterating on it in the future.

@s-hertel s-hertel Jul 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I still would prefer a separate configuration option altogether.
having a boolean option and another option later possibly to add other reporters seems like bad UI to me
I've held off on a couple minor ansible-galaxy features recently on similar concerns - too many people complain about the current user experience, so I don't want to paint us into more corners unless there's actually a group decision (the more people weighing in, the better imo).

webknjaz and others added 2 commits July 24, 2025 01:43
@webknjaz
Log every dependency resoltion step in debug mode
ec13d9e 
This idea is borrowed from what Pradyun added to pip's reporter for
resolvelib and adapted for `ansible-galaxy collection install`.
Normally, it uses a low-noise reporter but when debug is requested,
the verbose one is selected.
@webknjaz
📝 Add a change note for PR ansible#81693
acbcddf
@webknjaz
webknjaz force-pushed the features/galaxy-collection-depresolver-debugging-reporter branch from 92502be to acbcddf Compare July 23, 2025 23:45
webknjaz
webknjaz commented Jul 23, 2025
View reviewed changes
Comment thread changelogs/fragments/81693-ansible-galaxy-depresolver-verbose-reporter.yml

@webknjaz webknjaz Jul 23, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@s-hertel does this change note sound good to you?

@ansibot ansibot added stale_review Updates were made after the last review and the last review is more than 7 days old. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 23, 2025
@s-hertel
s-hertel dismissed their stale review July 24, 2025 14:59

I often want a way to inspect the dependency tree more easily, but I guess I misunderstood the purpose of this PR. I'm -1 on this as an internal tool.

@ansibot ansibot removed the stale_review Updates were made after the last review and the last review is more than 7 days old. label Jul 24, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 4, 2025
@sivel
sivel self-requested a review August 5, 2025 16:12
@webknjaz
webknjaz requested a review from nitzmahone August 5, 2025 16:39
@webknjaz

webknjaz commented Aug 5, 2025

Copy link
Copy Markdown
Member Author

It looks like most people lean towards having something like ANSIBLE_GALAXY_DEPENDENCY_RESOLVER_REPORTER: consice | debugging. So I'll be looking to disregard relying on ANSIBLE_DEBUG. Pending more reviews.

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s-hertel s-hertel s-hertel left review comments
@jborean93 jborean93 Awaiting requested review from jborean93
@sivel sivel Awaiting requested review from sivel
@nitzmahone nitzmahone Awaiting requested review from nitzmahone

At least 1 approving review is required to merge this pull request.

Assignees

@webknjaz webknjaz

Labels

affects_2.18 feature This issue/PR relates to a feature request. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@webknjaz @ansibot @s-hertel @nitzmahone @jborean93