fixes #67

[itdependsnetworks]

I am not sure populate_root is the best name, so welcome to hear suggestions. I updated the testing for similar tests.

[jvanderaa]

I want to say something like populate_facts or add_to_host_facts as part of it.

[itdependsnetworks]

I don't like using the term facts, since it would indicate the intention is to modify facts, when it is the root keys, I know there is a better term, but can't see it yet.

[pszulczewski]

I like the idea, but I wonder if Boolean will be sufficient?
Let's imagine two GQL queries, different filtering, but the same endpoint, what means that root elements are the same and when added with .get("data") one can overwrite the other.
Maybe instead of bool better to define key_name where the data would be added "to facts" under that key?

[itdependsnetworks]

I believe What you described is the default?

[pszulczewski]

OK, I see:
Beware, that the root keys provided by the query will overwrite any root keys already present, leverage the GraphQL alias feature to avoid issues.
So GQL alias is the recommended way to avoid overwrite, that's fine then. 👍

[FragmentedPacket]

I don't think we need the else here as we should be returning the data regardless. The flag should just set the facts or not.

[itdependsnetworks]

I had that at first, it seemed like duplicate, what say everyone else?

[jvanderaa]

Let's go ahead and remove the else, remove the indent for the results. Then the data key will be populated regardless, whether it has data or it is None.

[pszulczewski]

I want to say something like populate_facts or add_to_host_facts as part of it.

I don't like using the term facts, since it would indicate the intention is to modify facts, when it is the root keys, I know there is a better term, but can't see it yet.

populate_root might not be intuitive, can be received as permission escalation.

I would suggest:
populate_hostvars
add_to_hostvars

I like the second one, intuitive and shorter.

[pszulczewski]

Shouldn't that be boolean true ?

[itdependsnetworks]

In ansible they are equivalent

[pszulczewski]

Yes I know, unquoted yes is also evaluated as bool True.
This is more about documentation consistency, it's described as type bool in DOCUMENTATION section.
I was looking form a static-typing perspective, where bool is True or False without making assumptions about ansible internals, maybe unnecessarily. Anyway I don't have any issue with it, just a friendly suggestion.

[jvanderaa]

@FragmentedPacket can you take a look.