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

Use the full url to determine base url #45

Closed
wants to merge 1 commit into from

Conversation

SalDaniele
Copy link
Contributor

@SalDaniele SalDaniele commented Jul 5, 2023

As a result of f9f6a05, we are no longer using the full url
(https://{url}/redfish/v1/Systems/System.Embedded.1) to derive the base url, which causes self.baseurl to be set incorrectly to "://".

Use the updated self.url rather that url.

As a result of 36cfd96, we are no
longer using the full url
(https://{url}/redfish/v1/Systems/System.Embedded.1) to derive the base
url, which causes self.baseurl to be set incorrectly to "://".

Use the updated self.url rather that url.

Signed-off-by: Salvatore Daniele <sdaniele@redhat.com>
@SalDaniele
Copy link
Contributor Author

Hello, is it possible to get review on this fix? I believe there was a recent bug introduced that has broken our workflow which relies on this code.

i.e. When running insert_iso() we now get the following error

unknown url type: ':///redfish/v1/Managers/iDRAC.Embedded.1'

SalDaniele added a commit to SalDaniele/cluster-deployment-automation that referenced this pull request Jul 5, 2023
Typically we have been able to provide Redfish the iDrac ip alone.
However, a bug was introduced recently in aicli which causes
insert_iso() to fail due to self.baseurl being set incorrectly.

karmab/aicli#45

As a workaround, we can provide the full url instead.

Signed-off-by: Salvatore Daniele <sdaniele@redhat.com>
@karmab
Copy link
Owner

karmab commented Jul 10, 2023

my bad, I missed your fix and someone pinged me today on this, resulting on the same fix here at 297e9ce
thanks still!

@karmab karmab closed this Jul 10, 2023
@karmab
Copy link
Owner

karmab commented Jul 10, 2023

for the record, the benefit of this change is to remove the need for specifying bmc_model

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.

None yet

2 participants