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

pypuppetdb/types.py: Added noop attribute to Node and Report. #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakemalley
Copy link

Added an attribute noop to the Node and Report class types, so it is easy to ascertain whether the node was in noop for a particular report in the case of the Report type, or the last report run in the case of the Node type.

Added an attribute noop to the Node and Report class types, so it is easy to ascertain whether the node was in noop for a particular report in the case of the Report type, or the last report run in the case of the Node type.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 85.25% when pulling 29fdc64 on jakemalley:master into cedeecf on voxpupuli:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 85.25% when pulling 29fdc64 on jakemalley:master into cedeecf on voxpupuli:master.

@gdubicki
Copy link
Member

Hi @jakemalley ! Why do we need extra noop fields if in both of these types we already have status with the same information?

@jakemalley
Copy link
Author

Hello @gdubicki, it has been a long time since I've looked at this, but if memory serves it was due to status being set via:
'noop' if noop and noop_pending else status
and we wanted to report on whether a system was in 'noop' regardless of the noop_pending flag in certain situations. e.g. we needed to be able to query if the Puppet agent was in 'noop' even if there weren't any changes being applied to the box.

I think also in certain situations if there was a syntax error during catalogue compilation the status was failed (instead of noop) which was triggering false alerts which we wanted to ignore.

@gdubicki
Copy link
Member

gdubicki commented Mar 22, 2021

I had a feeling that this is because of the way we assign that status now. I am pretty new to the project so I have doubts about the approach we took too. Intuitively for me a run is a noop regardless if it resulted in pending changes or not.

Checking git blame for that 'noop' if noop and noop_pending else status line suggests that we should read #105 -> #103 to learn more about the rationale.

Full disclosure: I haven't done that yet. 😅

...but there seems to be a lot of discussion about a change introduces in PuppetDB 4.1 and being compatible with that. Puppet 4 EOLed in October 2018, so maybe we can simplify the code if we drop 4.x support completely. 🤔

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.

3 participants