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

FIX #35474 - resolve apt error messages #56179

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@KunyiLiu
Copy link

commented May 7, 2019

SUMMARY

FIX #35474

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/modules/packaging/os/apt.py

ADDITIONAL INFORMATION

kliu
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

@bcoca
Copy link
Member

left a comment

This just exchanges the issue to create the exact same problem when people use package

@samdoran

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@bcoca True, but I believe that we decided to make the code match the documentation when we went over this in triage. The idea being that the docs list name as the primary field which most people will probably use. That the error using package is confusing since most people may not realize it's an alias. Furthermore, most package modules use name for the package name. There are a couple that use package and pkg but most use name.

become: True
failed_when: False
register: apt_result
- name: Check that name and upgrade arguments produce a conflict error

This comment has been minimized.

Copy link
@samdoran

samdoran May 9, 2019

Member

Add a line between tasks to make them a bit more readable.

- name: Check that name and upgrade arguments produce a conflict error
assert:
that:
- >-

This comment has been minimized.

Copy link
@samdoran

samdoran May 9, 2019

Member

There's no need to use line folding here. You can just list the test cases:

Suggested change
- >-
- "parameters are mutually exclusive: deb|name|upgrade" in apt_result.msg
@@ -34,5 +34,7 @@
name: "{{ repodir }}"
state: absent

- include: 'apt-error-message.yml'

This comment has been minimized.

Copy link
@samdoran

samdoran May 9, 2019

Member

Rather than use include, use import_tasks. I know that is how the other tasks files are imported but they should be updated as well.

@bcoca

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@samdoran, still a better fix is to check if alias is set and use that in the error message (a reverse of current handle_aliases).

@ansibot ansibot added the stale_ci label May 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.