Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

raw_output_present fails when fact isn't explicitly in facts_to_collect #541

Closed
noahl opened this issue Dec 12, 2017 · 4 comments
Closed
Assignees

Comments

@noahl
Copy link

noahl commented Dec 12, 2017

The raw_output_present function starts like this:

   if this_fact not in fact_names:
        return {}, None

If the fact that it retrieves was not in the list of facts we need to collect, then return empty. I did this originally to avoid errors in the case where a fact was legitimately not collected.

However, some of our facts how have multiple triggers. karaf_home_bin_fuse can be collected if jboss.fuse-on-karaf.karaf-home is requested, or if the user requests jboss.fuse.summary, because it is used to generate the summary. If the user requests jboss.fuse.summary explicitly and not jboss.fuse-on-karaf.karaf-home, then raw_output_present will incorrectly return nothing when it should give the fact data.

@noahl
Copy link
Author

noahl commented Dec 12, 2017

Options for dealing with it:

  1. Give raw_output_present a complete list of facts requested that should make this fact be collected. This is correct, but now we have two lists to manually keep in sync, and they will inevitably get out of sync and cause errors or bugs when they do.

  2. Give up on this clause. If the fact data is present, always return it from raw_output_present. This will fix the current problem, but could expose hidden bugs. If there are any facts that collect when they are not supposed to, we need to filter them out before they are written to the csv or csv.DictWriter will error.

@noahl
Copy link
Author

noahl commented Dec 12, 2017

We could mitigate the issue with number 1 by fixing #464 at the same time, which would make the Python list the single, canonical list of fact dependencies and would prevent the class of errors I was worried about.

We could mitigate the issue with number 2 by blindly filtering every extra fact out just before we pass data to DictWriter.

I'm inclined to do number 1, because it moves the fingerprinter in a good long-term direction and shouldn't be too much extra work, but I would like more thoughts.

@chambridge
Copy link

I would suggest option 2. Its simple for now and doesn't add enhancements to rho. I think number 1 is good to keep in mind for sonar.

@noahl
Copy link
Author

noahl commented Dec 12, 2017

@chambridge okay, I will do number 2.

noahl added a commit that referenced this issue Dec 13, 2017
Make raw_output_present always return data if data is present
@noahl noahl self-assigned this Dec 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants