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

proxmox_template: fix the wrong path called on proxmox_template.task_status #9277

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

proxmox_template: fix the wrong path called on proxmox_template.task_status #9277

wants to merge 1 commit into from

Conversation

xunleii
Copy link

@xunleii xunleii commented Dec 18, 2024 •
edited
Loading

SUMMARY

The task_status functions uses the wrong Proxmox API path to fetch tasks logs (node instead of nodes). The documentation can be found here: https://pve.proxmox.com/pve-docs/api-viewer/index.html#/nodes/{node}/tasks (the node path doesn't exist)

Fixes #9276

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

proxmox_template

@xunleii xunleii changed the title proxmox_template: add missing 's' on proxmox_template.task_status proxmox_template: fix the wrong path called on proxmox_template.task_status Dec 18, 2024
@xunleii xunleii force-pushed the main branch 2 times, most recently from 2254068 to 956db7e Compare December 18, 2024 00:21
@ansibullbot
Copy link
Collaborator

cc @Ajpantuso @Thulium-Drake @UnderGreen @joshainglis @karmab @krauthosting
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Dec 18, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch backport-10 Automatically create a backport for the stable-10 branch labels Dec 20, 2024
felixfontein
felixfontein reviewed Dec 20, 2024
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! My guess is that the path changed in some Proxmox release. The module also needs to support Proxmox versions where it currently works, so you need to figure out in which version it changed and adjust the code to only use the new path for the appropriate versions.

...agments/9277-proxmox_template-fix-the-wrong-path-called-on-proxmox_template.task_status.yaml Outdated
@@ -0,0 +1,2 @@
bugfixes:
- proxmox_template - fix the wrong path called on proxmox_template.task_status (https://github.com/ansible-collections/community.general/pull/9277)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- proxmox_template - fix the wrong path called on proxmox_template.task_status (https://github.com/ansible-collections/community.general/pull/9277)
- proxmox_template - fix the wrong path called on ``proxmox_template.task_status`` (https://github.com/ansible-collections/community.general/issues/9276, https://github.com/ansible-collections/community.general/pull/9277).

@xunleii
Copy link
Author

xunleii commented Dec 21, 2024 •
edited
Loading

Thanks for your contribution! My guess is that the path changed in some Proxmox release. The module also needs to support Proxmox versions where it currently works, so you need to figure out in which version it changed and adjust the code to only use the new path for the appropriate versions.

Hello, thank you for your reply.

I hadn't thought to do any research, thinking it was a typo 😅.

However, after some research, and as far as I can go, the API path to interact with a node has always been nodes and not node.

With all this in mind, I think it's a typo 😟.

@xunleii xunleii force-pushed the main branch 3 times, most recently from 6d82eb2 to 9d5fd39 Compare December 21, 2024 08:26
@xunleii @felixfontein
fix: add missing 's' on proxmox_template.task_status
0e44300 
Missing the 's' means using the wrong API, making log reading
impossible. Should fix !9276

Co-Authored-By: Felix Fontein <[email protected]>
Signed-off-by: Alexandre Nicolaie <[email protected]>
@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
backport-9 Automatically create a backport for the stable-9 branch backport-10 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. ci_verified Push fixes to PR branch to re-run CI module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

proxmox_template: 501 Not Implemented when using the uri field
3 participants
@xunleii @ansibullbot @felixfontein