Fix for iptables states fail when state_aggregate option is enabled #54081
Conversation
…ion to util, added docstrings
ghost
requested a review
from 
xeacott
changed the title
|
Forgot to squash when I merged 👎 |
|
I'm not entirely sure that I like just removing circular refs. I mean, it probably solves the problem, but it obscures the fact that we shouldn't be creating circular data structures in the first place. At least, I'm not aware of any place in salt where that would be a good thing to have 🤷♂ |
|
I agree. This just puts in a fail safe that if we do have a circular reference, clean it up and let it pass through. However, I'm not entirely sure where an object would have gained a circular reference, which would be the real root cause. I can investigate it further for sure! |
|
I've noticed that we have occasions where this is the case. I wouldn't be opposed to putting this PR in if we also logged (probably as an error, probably with the stack trace?) that a circular reference was detected, asking users to report it. At least, I don't think I would 😂 |
|
Let me find the correct place to log and I'll get that in with this PR! |
| test_list = {'foo': [],} | ||
| test_list['foo'].append((test_list,)) | ||
| ret = salt.utils.data._remove_circular_refs(ob=test_list) | ||
| self.assertDictEqual(ret, {'foo': [(None,)]}) |
There was a problem hiding this comment.
Looks like this is failing lint. You need a space after the comma.
What does this PR do?
Add method to remove circular references to objects that must be decoded/encoded.
What issues does this PR fix or reference?
Fix #53353 by cleaning data object in case of circular references.
Tests written?
Yes
Commits signed with GPG?
No