search

openbmc / openbmc-test-automation

Apache License 2.0
100 stars 92 forks source link

lib/bmc_redfish_utils.py __init__ create a redfish session, but not logout #2214

Closed IdleIdiot closed 10 months ago

IdleIdiot commented 11 months ago

When I Use:

Library bmc_redfish_utils.py WITH NAME redfish_utils

This will create a bmc_redfish_utils instance, and use init method to verify redfish interface.

But its not logout after execute. image

So when i execute all cases, I find some extra sessions in my web UI.

I modify this file like this now: image

gkeishin commented 11 months ago

@adathatri can u respond to this query ?

adathatri commented 11 months ago

The issue is that the existing code is leaving logged in sessions without cleaning them up. @tester-noob Can you please push your changes to the repository and then we can take it from there. Thanks.

SrideviRamesh commented 11 months ago

What if the user does not want to logout immediately. Should we provide an optional parameter/Flag if logout is required or not ?

IdleIdiot commented 11 months ago

The issue is that the existing code is leaving logged in sessions without cleaning them up. @tester-noob Can you please push your changes to the repository and then we can take it from there. Thanks.

I create a pull requests and commit my changes now. Isn't that okay

adathatri commented 11 months ago

The issue is that the existing code is leaving logged in sessions without cleaning them up. @tester-noob Can you please push your changes to the repository and then we can take it from there. Thanks.

I create a pull requests and commit my changes now. Isn't that okay

Can you share the link? The changes should be pushed to the repo in Gerrit- https://gerrit.openbmc.org/q/project:openbmc%252Fopenbmc-test-automation

adathatri commented 11 months ago

What if the user does not want to logout immediately. Should we provide an optional parameter/Flag if logout is required or not ?

The framework is designed in such a way that an exclusive login and logout is required in the test suite. This code is just to check the response and should not be relied for any other use case. So logging out looks good here.

IdleIdiot commented 11 months ago

The issue is that the existing code is leaving logged in sessions without cleaning them up. @tester-noob Can you please push your changes to the repository and then we can take it from there. Thanks.

I create a pull requests and commit my changes now. Isn't that okay

Can you share the link? The changes should be pushed to the repo in Gerrit- https://gerrit.openbmc.org/q/project:openbmc%252Fopenbmc-test-automation

https://gerrit.openbmc.org/c/openbmc/openbmc-test-automation/+/66632

gkeishin commented 11 months ago

@Nevermore1849

For any code changes to get merged, 

OpenBMC contributors are required to execute an OpenBMC CLA (Contributor License Agreement) 
before their contributions can be accepted.

I don't see you are listed in the CLA here https://drive.google.com/drive/folders/1Ooi0RdTcaOWF1DWFJUAJDdN7tRKde7Nl

Can you point me out if you are already part of it. If you are new, take a look at this one-time process https://github.com/openbmc/docs/blob/master/CONTRIBUTING.md#starting-out

gkeishin commented 10 months ago

Please let us know if we need any new information before we close this ticket. Thanks