Prevent double authentication while using openqaoa-azure

TerraVenil commented 1 year ago

Description

This PR is for solving issue https://github.com/entropicalabs/openqaoa/issues/233.

Checklist

[x] I have performed a self-review of my code.
[ ] I have commented my code and used numpy-style docstrings
[ ] I have made corresponding updates to the documentation.
[x] My changes generate no new warnings
[ ] I have added/updated tests to make sure bugfix/feature works.
[x] New and existing unit tests pass locally with my changes.

Type of change

Please delete options that are not relevant.

[x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

By running tests locally for openqaoa-azure and openqaoa-core.

shahidee44 commented 1 year ago

@TerraVenil You will need to update this PR with the latest version of dev so that it can pass the relevant unittests. :)

TerraVenil commented 1 year ago

It does not account for any other case, for example, cases where there are incorrect credentials or names. In those cases, the check_connection method is run again for the same device.

Hi @shahidee44. Thank you for your comments to PR. Please take a look my comment here https://github.com/entropicalabs/openqaoa/issues/233#issuecomment-1580713681, eventually if you have any issues with authentication after timeout (default value is 5 mins) authenticator will throw an error that will stop any further processing. That means it wouldn't execute check_connection twice. But in general it's possible to improve handling of any authentication errors from Azure authenticator by handlingClientAuthenticatorError but from my point of view authenticator throws already user friendly errors.

shahidee44 commented 1 year ago

Thats a good point on Authentication Errors. The intention of the Device object is to catch Exception (s) that occur during the authentication process and just return a boolean based on the success of the authentication. The handling of the error would be done by other layers controlling the Device object. This was done to ensure consistency in error handling across the different Devices.

Also, it seems like I'm made a mistake in my initial comment, the new amendment only authenticates iff the Device object has not tried to authenticate before. The booleans provider_connected and qpu_connected change depending on whether certain parts of the authentication process is successful.

It is then more appropriate to note that:

If a Device Object has already tried to be authenticated before passing through to the Backend. Then, there will be no attempt to reauthenticate.

This is an important point to note since in some fringe case where the user authenticates, initializes the Device and runs check_connection, makes some change to the internal attributes to the Device after the fact, then passes that modified Device into the Backend. The Device object will not be reauthenticated.

The main point here would be that we need to let users know that the Device object would need to be recreated or check_connection has to be run again on the same object, if the user is looking to reauthenticate the Device.

entropicalabs / openqaoa