Redfish modules: rename _facts -> _info

[felixfontein]

SUMMARY

Fixes #60523.

ISSUE TYPE

• Bugfix Pull Request
• New Module Pull Request

COMPONENT NAME

redfish_facts
idrac_redfish_facts

[ansibot]

cc @acozine @billdodd @jose-delarosa @mraineri @tomasg2012
click here for bot help

[felixfontein]

ready_for_review

[billdodd]

I don't like that this leaves us with two copies of essentially the same module that must be maintained going forward. Can't this be done in a way that we have, for example, a redfish_info.py module and the redfish_facts.py module just calls the redfish_info.py module? Similar to how the cloudformation_info.py <-> cloudformation_facts.py modules are handled in #60178?

[felixfontein]

@billdodd the difference to cloudformation_info/facts is that there, the return value's name did not contain facts. Here, the return variable name is called redfish_facts (in both modules). Of course, you can have a _info module for which you have to use result.redfish_facts if you prefer that, I assumed (since there was no reaction to #60523) that there was no preference and chose the nicer return name.

[felixfontein]

@billdodd so what do you prefer?
a) deprecate _facts and add info (this PR now);
b) rename module and keep _facts in return value name (even though it is no longer ansible_facts);
c) something else?

There's also the possibility of renaming the module and making the return value name dependent on whether the module is called as _info or _facts. I personally don't think it's a good solution, since the auto-generated documentation is less clear, but if you really want this, we can also do that. This is what the Hetzner cloud team did (see here).

[billdodd]

@felixfontein Thanks for the details and options. I’m away from my computer today, but will review and respond tomorrow.

[billdodd]

@felixfontein:

My original thought was to do this option that you described:

There's also the possibility of renaming the module and making the return value name dependent on whether the module is called as _info or _facts.

But I didn't consider the auto-generated documentation issue you mentioned.

Given that, it seems like the better option would be your option (b):

b) rename module and keep _facts in return value name (even though it is no longer ansible_facts);

I really want to avoid having both redfish_facts and redfish_info modules side-by-side that both need to be maintained (duplicate code). If I understand correctly, your option (b) does avoid that situation. Do I understand correctly?

BTW - I don't really have a preference or issue regarding whether the return value name is "redfish_facts" vs. "redfish_info". So whichever of those you think makes the most sense for Ansible in this situation is fine with me.

[felixfontein]

Given that, it seems like the better option would be your option (b):

b) rename module and keep _facts in return value name (even though it is no longer ansible_facts);

I really want to avoid having both redfish_facts and redfish_info modules side-by-side that both need to be maintained (duplicate code). If I understand correctly, your option (b) does avoid that situation. Do I understand correctly?

Yes, that's correct. There is one copy of the module, and it handles both its names slightly differently. (And the _facts name is deprecated and will vanish in 4 versions.)

BTW - I don't really have a preference or issue regarding whether the return value name is "redfish_facts" vs. "redfish_info". So whichever of those you think makes the most sense for Ansible in this situation is fine with me.

I would say anything but redfish_facts :-) But with option b) from above, it has to be that name.

[billdodd]

Yes, that's correct. There is one copy of the module, and it handles both its names slightly differently. (And the _facts name is deprecated and will vanish in 4 versions.)

Sounds good!

I would say anything but redfish_facts :-) But with option b) from above, it has to be that name.

Got it. :-) Thanks again for your help.

[felixfontein]

Ok. I'll adjust the PR next week (probably Monday).

[felixfontein]

I rebased (to remove conflict) and then added a commit which changes the PR to the rename method. PTAL!

[samccann]

Docs portion LGTM

[billdodd]

I reviewed the changes and did a test with the old module name and the new module name. All LGTM.

Thanks @felixfontein

shipit

[felixfontein]

Great! Thanks a lot for reviewing and testing, @billdodd!