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

Fixes #943: Partial update of custom fields is not idempotent #944

Conversation

cfiehe
Copy link
Contributor

@cfiehe cfiehe commented Feb 12, 2023

It is important that the object's custom fields dictionary contains only those keys that were given by the user. Otherwise the following dictionary comparison does not work and leads to the result, that there is a difference in state even when the entries already have the desired state. The issue is caused, because the comparison takes also those entries into account that were not given by the user.

Signed-off-by: Christoph Fiehe c.fiehe@eurodata.de

Related Issue

#943: Partial update of custom fields is not idempotent

New Behavior

Partial updates of custom fields are now idempotent.

Contrast to Current Behavior

This fix corrects the misleading diff report and ensures idempotency when only some custom fields are updated.

Discussion: Benefits and Drawbacks

This fix is fully backward-compatible and ensures idempotency in case of custom field updates.

Double Check

  • I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR targets the devel branch.

@cfiehe cfiehe force-pushed the fix_partial_update_of_custom_fields_is_not_idempotent branch 3 times, most recently from fbf7d17 to 5b5b830 Compare February 12, 2023 16:52
@sc68cal
Copy link
Contributor

sc68cal commented Feb 12, 2023

Would appreciate the addition of unit tests based on your bug description in the issue

@cfiehe cfiehe force-pushed the fix_partial_update_of_custom_fields_is_not_idempotent branch 2 times, most recently from 839e719 to 7132745 Compare February 13, 2023 07:36
…empotent

It is important that the object's custom fields dictionary contains only those keys that were given by the user. Otherwise the following dictionary comparison does not work and leads to the result, that there is a difference in state even when the entries already have the desired state. The issue is caused, because the comparison takes also those entries into account that were not given by the user.

Signed-off-by: Christoph Fiehe <c.fiehe@eurodata.de>
@cfiehe cfiehe force-pushed the fix_partial_update_of_custom_fields_is_not_idempotent branch from 7132745 to c3fb7b9 Compare February 13, 2023 08:38
@cfiehe
Copy link
Contributor Author

cfiehe commented Feb 13, 2023

I have modified the test cases. Maybe we must increase the test scope in the future and add dedicated tests for custom field handling.

@rodvand rodvand merged commit 797c383 into netbox-community:devel Feb 13, 2023
rodvand pushed a commit to rodvand/ansible_modules that referenced this pull request Feb 24, 2023
…empotent (netbox-community#944)

It is important that the object's custom fields dictionary contains only those keys that were given by the user. Otherwise the following dictionary comparison does not work and leads to the result, that there is a difference in state even when the entries already have the desired state. The issue is caused, because the comparison takes also those entries into account that were not given by the user.

Signed-off-by: Christoph Fiehe <c.fiehe@eurodata.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants