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

ref: Migrate profile_info to use common info module #558

Merged

Conversation

yec-akamai
Copy link
Contributor

📝 Description

Migrate profile_info to use common info module. Also make some necessary change to the base info module because profile_info doesn't require any attribute or parameter to call.

✔️ How to Test

make TEST_ARGS="-v profile_info" test

Manual test:

  1. In a sandbox environment, run the following script to get the info of current profile:
- name: test
  hosts: localhost
  tasks:
    - name: Get profile_info about current profile
      linode.cloud.profile_info:
      register: profile
  1. Observe that profile info the successfully returned.

@yec-akamai yec-akamai added the improvement for improvements in existing functionality in the changelog. label Aug 9, 2024
@yec-akamai yec-akamai requested a review from a team as a code owner August 9, 2024 19:13
@yec-akamai yec-akamai requested review from jriddle-linode and ezilber-akamai and removed request for a team August 9, 2024 19:13
Comment on lines -223 to +243
attribute_names = [v.name for v in self.attributes]
attribute_names = (
[[v.name for v in self.attributes]]
if len(self.attributes) > 0
else None
)

super().__init__(
module_arg_spec=self.module_arg_spec,
required_one_of=[attribute_names],
mutually_exclusive=[attribute_names],
required_one_of=attribute_names,
mutually_exclusive=attribute_names,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous implementation brought an edge case that when the module didn't require any attribute, we still send an [[]] value as required_one_of. It caused a bug to fail the module.

Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Integration and manual tests working properly. Nice work!

Copy link
Collaborator

@jriddle-linode jriddle-linode left a comment

Choose a reason for hiding this comment

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

LGTM works locally.

@yec-akamai yec-akamai merged commit 217b05f into linode:dev Aug 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement for improvements in existing functionality in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants