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 an issue retrieving some types of 1Password items. (#47213) #50160

Open
wants to merge 3 commits into
base: stable-2.7
from

Conversation

Projects
None yet
4 participants
@Rylon
Copy link
Contributor

Rylon commented Dec 19, 2018

  • Some types of 1Password items have a 'password' field alongside the 'fields' attribute, not inside it, so we need to search there as well.

  • Add changelog fragement for onepassword_facts PR #47213.

(cherry picked from commit aacd22a)

SUMMARY

This is the requested backport pull request for #47213.

ISSUE TYPE
  • Backport Pull Request
COMPONENT NAME

onepassword_facts

Fix an issue retrieving some types of 1Password items. (#47213)
* Some types of 1Password items have a 'password' field alongside the 'fields' attribute, not inside it, so we need to search there as well.

* Add changelog fragement for onepassword_facts PR #47213.

(cherry picked from commit aacd22a)
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 19, 2018

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/identity/onepassword_facts.py:160:18: undefined-variable Undefined variable 'AnsibleModuleError'
lib/ansible/modules/identity/onepassword_facts.py:160:37: undefined-variable Undefined variable 'to_native'

click here for bot help

@Rylon

This comment has been minimized.

Copy link
Contributor

Rylon commented Dec 19, 2018

@samdoran I'm not sure about this error, as these should be available from Ansible itself?

@ansibot ansibot added the stale_ci label Dec 27, 2018

@abadger

This comment has been minimized.

Copy link
Member

abadger commented Jan 7, 2019

For AnsibleModuleError, you'd have to create that in your module as it's not defined for ansible modules. (Remember that Ansible modules are little scripts that get transfered to the remote machine and run so they only have access to other things inside of ansible/module_utils when they run).

For to_native, you need to add an import statement to make that work.

It looks like these problems are being introduced when resolving conflicts with this commit: b608543 Some more work needs to be done to reconcile the code before this backport is ready.

@ansibot ansibot removed the needs_triage label Jan 7, 2019

@Rylon

This comment has been minimized.

Copy link
Contributor

Rylon commented Jan 7, 2019

Thanks @abadger I think the conflict has probably arisen because the branches diverged as the main PR was open for a while, but it’s now been merged into devel, so I’m not sure what the best action to take is here. If this can’t be cleanly merged for the backport branch, should I just leave it and it’ll ultimately get shipped in a later version?

Apologies for the confusion as I’ve not done one of these before.

@abadger

This comment has been minimized.

Copy link
Member

abadger commented Jan 7, 2019

It's up to you. If you're okay with the change not making it in until 2.8.0 is released, you can close this backport pull request and we'll just continue to live with the bug in the 2.7.x series.

@Rylon

This comment has been minimized.

Copy link
Contributor

Rylon commented Jan 8, 2019

@abadger I guess it's a minor issue so it could wait until 2.8, or I can try again to cherry-pick a particular version of the module from devel, rather than the one from the original PR, would that perhaps work better? So essentially taking the latest working version from devel?

@abadger

This comment has been minimized.

Copy link
Member

abadger commented Jan 9, 2019

@Rylon We'd really rather have the version in 2.7 have commits associated with devel. That could mean cherry-picking all of the commits from devel for this module to the stable-2.7 branch, though. However, previous commits might not have been backported already because they were deemed to be features or too intrusive and potentially destabilizing. Those would still not be allowed to be backported. You would need to talk to the maintainers of the module (or perhaps the people who created the commits in devel which haven't yet been backported) to figure out if that's the problem or not.

@Rylon

This comment has been minimized.

Copy link
Contributor

Rylon commented Jan 10, 2019

Hi @samdoran do you have any suggestions as for the best thing for me to do here please? Thanks 👍

@samdoran

This comment has been minimized.

Copy link
Member

samdoran commented Jan 14, 2019

@Rylon Since backporting #45427 is too big a change, I would just add the relevant blocks of code that do exist in devel to this backport PR unless @abadger objects.

Importing to_native and defining AnsibleModuleError should do the trick. It's not a "clean" backport, but at least it does contain code that exists in devel.

@abadger

This comment has been minimized.

Copy link
Member

abadger commented Jan 14, 2019

@Rylon

This comment has been minimized.

Copy link
Contributor

Rylon commented Jan 15, 2019

Thanks @abadger and @samdoran I did that and looks like we're all good now 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment