Fix optional_claims handling for azure_rm_adapplication #1480

kent007 · 2024-03-05T17:30:05Z

SUMMARY
Fixes #1475

ISSUE TYPE
Bugfix Pull Request

COMPONENT NAME
azure_rm_adapplication

ADDITIONAL INFORMATION
I copied the format already present for manipulating other complex fields. Conceivably this could be divided into 3 module level args instead of a single complex arg, but I don't see any direct benefit for doing so. Tests seem to work okay against my azure instance.

Previously filed in #1479 and #1476, closed those while I was reworking branches on my fork

plugins/modules/azure_rm_adapplication.py

This reverts commit a1ec38a.

Fred-sun

ERROR: plugins/modules/azure_rm_adapplication.py:171:111: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:179:118: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:198:116: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:203:111: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:211:112: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:230:116: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:235:118: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:243:112: W291: trailing whitespace

plugins/modules/azure_rm_adapplication.py

Fred-sun

line 172,180,199,204,212,231,236,244

plugins/modules/azure_rm_adapplication.py

Fred-sun · 2024-03-19T00:27:46Z

@kent007 Please remove trailing space

ERROR: plugins/modules/azure_rm_adapplication.py:171:111: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:179:118: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:198:116: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:203:111: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:211:112: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:230:116: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:235:118: W291: trailing whitespace
ERROR: plugins/modules/azure_rm_adapplication.py:243:112: W291: trailing whitespace

Fred-sun

Define errors!

plugins/modules/azure_rm_adapplication.py

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

plugins/modules/azure_rm_adapplication.py

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

# Conflicts: # plugins/modules/azure_rm_adapplication.py

kent007 · 2024-03-20T13:23:46Z

After merging all suggested changes and resolving merge conflicts, I still see the following pep8 errors from sanity testing:

ERROR: Found 3 pep8 issue(s) which need to be resolved:
ERROR: plugins/modules/azure_rm_adapplication.py:171:161: E501: line too long (191 > 160 characters)
ERROR: plugins/modules/azure_rm_adapplication.py:200:161: E501: line too long (191 > 160 characters)
ERROR: plugins/modules/azure_rm_adapplication.py:229:161: E501: line too long (191 > 160 characters)

Should we address these?

Fred-sun · 2024-03-20T23:45:12Z

@kent007 Yes, changes are required, documentation must be pep8 compliant! Thank you!

plugins/modules/azure_rm_adapplication.py

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Fred-sun · 2024-03-22T07:51:03Z

@kent007 I most made some changes to this PR(kent007#1) and submit your repo which you can merge into your branch 'dev-application'. The changes are as follows:

The parameters of the test cases using the wrong - tests/integration/the targets/azure_rm_adapplication/tasks/main yml
Add the return value 'optional_claims' to azure_rm_adapplication_info.py
Change the type of parameter additional_properties in azure_rm_adapplication.py to list

Add 'optional_claims' to the return value of azure_rm_adapplication

kent007 · 2024-03-22T13:13:31Z

@kent007 I most made some changes to this PR(kent007#1) and submit your repo which you can merge into your branch 'dev-application'. The changes are as follows:

The parameters of the test cases using the wrong - tests/integration/the targets/azure_rm_adapplication/tasks/main yml

Add the return value 'optional_claims' to azure_rm_adapplication_info.py

Change the type of parameter additional_properties in azure_rm_adapplication.py to list

I see, thank you! Merged

Fred-sun

A small error, return value duplicate, please delete. Thank you!

plugins/modules/azure_rm_adapplication.py

…ections#1480) * Fix: parse paginated replies for users and groups * Fix: await * wrong var return * Fix: optional claims for applications * Chore: correct documentation * Fix: check for optional_claims being None * Address PR feedback * Revert "Address PR feedback" This reverts commit a1ec38a. * Address PR feedback * Fix: line endings * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Fix: more doc formatting * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Update plugins/modules/azure_rm_adapplication.py Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * add `_claims` suffix to all three new parameters * Apply suggestions from code review Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> * Add 'optional_claims' to the return value of azure_rm_adapplication * Modify 'additional_properties' type to 'list' * remove duplicate keys --------- Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com> Co-authored-by: Fred-sun <xiuxi.sun@qq.com>

kent007 and others added 7 commits February 15, 2024 15:16

Fix: parse paginated replies for users and groups

dda1c7e

Fix: await

0725704

wrong var return

d4289c0

Merge branch 'ansible-collections:dev' into dev

f911b9f

Fix: optional claims for applications

e394873

Merge remote-tracking branch 'origin/dev' into dev

e78208c

Chore: correct documentation

e0c3612

Fred-sun added medium_priority Medium priority new_feature New feature requirments work in In trying to solve, or in working with contributors enhancement New feature or request labels Mar 6, 2024

Fix: check for optional_claims being None

85096af

Fred-sun reviewed Mar 14, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

kent007 added 3 commits March 15, 2024 10:16

Address PR feedback

a1ec38a

Revert "Address PR feedback"

bbe53f6

This reverts commit a1ec38a.

Address PR feedback

7092c15

Fred-sun reviewed Mar 18, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

Fred-sun reviewed Mar 18, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

Fix: line endings

bd84d9f

Fred-sun reviewed Mar 19, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

kent007 and others added 2 commits March 19, 2024 09:18

Update plugins/modules/azure_rm_adapplication.py

9a89bb0

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Fix: more doc formatting

766f3b2

Fred-sun reviewed Mar 20, 2024

View reviewed changes

kent007 and others added 6 commits March 20, 2024 09:09

Update plugins/modules/azure_rm_adapplication.py

2ce4fcb

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

040b3b6

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

c00bc83

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

2f99856

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

6d60d25

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

95a3caf

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

kent007 and others added 8 commits March 20, 2024 09:10

Update plugins/modules/azure_rm_adapplication.py

31ce327

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

b878315

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

9a167bd

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

30e7d5a

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Update plugins/modules/azure_rm_adapplication.py

e49c608

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Apply suggestions from code review

f9f3bc3

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

add _claims suffix to all three new parameters

c11322f

Merge branch 'dev' into dev-applications

b06e3bd

# Conflicts: # plugins/modules/azure_rm_adapplication.py

Fred-sun reviewed Mar 21, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

kent007 and others added 3 commits March 21, 2024 09:56

Apply suggestions from code review

7669ca0

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>

Add 'optional_claims' to the return value of azure_rm_adapplication

8ff40b3

Modify 'additional_properties' type to 'list'

a5001c4

Merge pull request #1 from Fred-sun/update-kent007-dev-application

49c31ef

Add 'optional_claims' to the return value of azure_rm_adapplication

Fred-sun reviewed Mar 25, 2024

View reviewed changes

plugins/modules/azure_rm_adapplication.py Outdated Show resolved Hide resolved

remove duplicate keys

ad89849

Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Mar 26, 2024

Fred-sun assigned xuzhang3 Mar 26, 2024

xuzhang3 approved these changes May 21, 2024

View reviewed changes

xuzhang3 merged commit e8ce7e1 into ansible-collections:dev May 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix optional_claims handling for azure_rm_adapplication #1480

Fix optional_claims handling for azure_rm_adapplication #1480

kent007 commented Mar 5, 2024

Fred-sun left a comment

Fred-sun left a comment

Fred-sun commented Mar 19, 2024

Fred-sun left a comment

kent007 commented Mar 20, 2024

Fred-sun commented Mar 20, 2024

Fred-sun commented Mar 22, 2024 •

edited

Loading

kent007 commented Mar 22, 2024

Fred-sun left a comment

Fix optional_claims handling for azure_rm_adapplication #1480

Fix optional_claims handling for azure_rm_adapplication #1480

Conversation

kent007 commented Mar 5, 2024

Fred-sun left a comment

Choose a reason for hiding this comment

Fred-sun left a comment

Choose a reason for hiding this comment

Fred-sun commented Mar 19, 2024

Fred-sun left a comment

Choose a reason for hiding this comment

kent007 commented Mar 20, 2024

Fred-sun commented Mar 20, 2024

Fred-sun commented Mar 22, 2024 • edited Loading

kent007 commented Mar 22, 2024

Fred-sun left a comment

Choose a reason for hiding this comment

Fred-sun commented Mar 22, 2024 •

edited

Loading