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

Refactor service_root variable #56198

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@billdodd
Copy link
Contributor

commented May 8, 2019

SUMMARY

Small enhancement includes the following changes:

  1. Refactor to move the definition of the service root variable (value "/redfish/v1/") out of the individual modules and into the redfish_utils.py utility module. Service root is a Redfish protocol defined URI so defining it once in the utility module that needs it makes more sense that defining in every one of the top-level Redfish modules.

  2. Added a module param to the RedfishUtils.__init__() constructor so that the module instance will be available in the redfish_utils.py utility module. This allows the utility module to make calls like module.deprecate(), module.log(), etc.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

redfish_utils.py
redfish_facts.py
redfish_config.py
redfish_command.py
idrac_redfish_facts.py
idrac_redfish_config.py
idrac_redfish_command.py

ADDITIONAL INFORMATION
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

@billdodd billdodd force-pushed the billdodd:feature/refactor-service-root branch from b8fc377 to 699828c May 14, 2019

@billdodd

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

ready_for_review

@jose-delarosa

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

@billdodd Tested it and looks good, thank you.

Good idea on point 1, makes sense. On point 2, should be a good idea too, but currently not being used. Does it make sense to wait and add it until it's actually used? Do you have a PR coming soon that will use the module instance?

@ansibot ansibot removed the needs_triage label May 16, 2019

@jose-delarosa

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

Had an offline discussion on point 2, looks good.

shipit

@ansibot ansibot added shipit and removed community_review labels May 16, 2019

@mraineri

This comment has been minimized.

Copy link

commented May 20, 2019

shipit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.