Repository: Qiskit/qiskit-ibm-runtime
Pull request #125 (2022-01-26 11:36:41)

Improve account management error handling
### Summary
- avoid that an existing account is overwritten unless explicitly specified (raise `AccountAlreadyExistsError` otherwise)
- raise dedicated error classes in other account management related failure cases, i.e.
  - `InvalidAccountError` instead of `ValueError`
  - `AccountNotFoundError` instead of `ValueError`

### Details and comments
Fixes https://github.com/Qiskit/qiskit-ibm-runtime/issues/117 and implements related improvements.

### Follow-Up
- [X] let Beccy now so she can update the docs accordingly 
   - reached out to Beccy and @rathishcholarajan 

daka1510: @rathishcholarajan all failing push tests show
```
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='us-east.quantum-computing.us-east.quantum-computing.cloud.ibm.com', port=443): Max retries exceeded with url: /devices (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x133172f10>: Failed to establish a new connection: [Errno 8] nodename nor servname provided, or not known'))
```
I.e. I missed to update the Cloud URL to "https://cloud.ibm.com" in my fork (done now).

Do you want me to push a dummy commit to trigger the push tests again or can we go ahead and integrate it given that the changes in this PR are very unlikely to cause issues in these tests?
daka1510: @rathishcholarajan updated the wrong setting and push tests still failed. Gave it another try now :).

Patch:
diff --git a/README.md b/README.md
index 86c91d0c..dbfc3fa2 100644
--- a/README.md
+++ b/README.md
@@ -1,5 +1,4 @@
 # Qiskit IBM Runtime
-
 [![License](https://img.shields.io/github/license/Qiskit/qiskit-ibm-runtime.svg?style=popout-square)](https://opensource.org/licenses/Apache-2.0)[![Push-Test](https://github.com/Qiskit/qiskit-ibm-runtime/actions/workflows/main.yml/badge.svg)](https://github.com/Qiskit/qiskit-ibm-runtime/actions/workflows/main.yml)[![](https://img.shields.io/github/release/Qiskit/qiskit-ibm-runtime.svg?style=popout-square)](https://github.com/Qiskit/qiskit-ibm-runtime/releases)[![](https://img.shields.io/pypi/dm/qiskit-ibm-runtime.svg?style=popout-square)](https://pypi.org/project/qiskit-ibm-runtime/)[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)
 
 
@@ -191,4 +190,4 @@ use the `Qiskit` tag in [Stack Exchange].
 [many people]: https://github.com/Qiskit/qiskit-ibm-runtime/graphs/contributors
 [BibTeX file]: https://github.com/Qiskit/qiskit/blob/master/Qiskit.bib
 [Apache License 2.0]: https://github.com/Qiskit/qiskit-ibm-runtime/blob/main/LICENSE.txt
-[tutorials]: https://github.com/Qiskit/qiskit-ibm-runtime/tree/main/docs/tutorials
\ No newline at end of file
+[tutorials]: https://github.com/Qiskit/qiskit-ibm-runtime/tree/main/docs/tutorials
diff --git a/qiskit_ibm_runtime/accounts/__init__.py b/qiskit_ibm_runtime/accounts/__init__.py
index ea424ef4..b44e1a3c 100644
--- a/qiskit_ibm_runtime/accounts/__init__.py
+++ b/qiskit_ibm_runtime/accounts/__init__.py
@@ -16,3 +16,8 @@ Account management functionality related to the IBM Runtime Services.
 
 from .account import Account, AccountType
 from .management import AccountManager
+from .exceptions import (
+    AccountNotFoundError,
+    AccountAlreadyExistsError,
+    InvalidAccountError,
+)
diff --git a/qiskit_ibm_runtime/accounts/account.py b/qiskit_ibm_runtime/accounts/account.py
index faa1fa55..6ff53c7b 100644
--- a/qiskit_ibm_runtime/accounts/account.py
+++ b/qiskit_ibm_runtime/accounts/account.py
@@ -18,9 +18,11 @@ from urllib.parse import urlparse
 
 from requests.auth import AuthBase
 from typing_extensions import Literal
+
+from .exceptions import InvalidAccountError
+from ..api.auth import LegacyAuth, CloudAuth
 from ..proxies import ProxyConfiguration
 from ..utils.hgp import from_instance_format
-from ..api.auth import LegacyAuth, CloudAuth
 
 AccountType = Optional[Literal["cloud", "legacy"]]
 
@@ -104,7 +106,7 @@ class Account:
         """Validates the account instance.
 
         Raises:
-            ValueError: if the account is invalid
+            InvalidAccountError: if the account is invalid
 
         Returns:
             This Account instance.
@@ -121,7 +123,7 @@ class Account:
     def _assert_valid_auth(auth: AccountType) -> None:
         """Assert that the auth parameter is valid."""
         if not (auth in ["cloud", "legacy"]):
-            raise ValueError(
+            raise InvalidAccountError(
                 f"Invalid `auth` value. Expected one of ['cloud', 'legacy'], got '{auth}'."
             )
 
@@ -129,7 +131,7 @@ class Account:
     def _assert_valid_token(token: str) -> None:
         """Assert that the token is valid."""
         if not (isinstance(token, str) and len(token) > 0):
-            raise ValueError(
+            raise InvalidAccountError(
                 f"Invalid `token` value. Expected a non-empty string, got '{token}'."
             )
 
@@ -139,7 +141,9 @@ class Account:
         try:
             urlparse(url)
         except:
-            raise ValueError(f"Invalid `url` value. Failed to parse '{url}' as URL.")
+            raise InvalidAccountError(
+                f"Invalid `url` value. Failed to parse '{url}' as URL."
+            )
 
     @staticmethod
     def _assert_valid_proxies(config: ProxyConfiguration) -> None:
@@ -152,7 +156,7 @@ class Account:
         """Assert that the instance name is valid for the given account type."""
         if auth == "cloud":
             if not (isinstance(instance, str) and len(instance) > 0):
-                raise ValueError(
+                raise InvalidAccountError(
                     f"Invalid `instance` value. Expected a non-empty string, got '{instance}'."
                 )
         if auth == "legacy":
@@ -160,6 +164,6 @@ class Account:
                 try:
                     from_instance_format(instance)
                 except:
-                    raise ValueError(
+                    raise InvalidAccountError(
                         f"Invalid `instance` value. Expected hub/group/project format, got {instance}"
                     )
diff --git a/qiskit_ibm_runtime/accounts/exceptions.py b/qiskit_ibm_runtime/accounts/exceptions.py
index bd20386f..c8556761 100644
--- a/qiskit_ibm_runtime/accounts/exceptions.py
+++ b/qiskit_ibm_runtime/accounts/exceptions.py
@@ -17,3 +17,15 @@ from ..exceptions import IBMError
 
 class AccountsError(IBMError):
     """Base class for errors raised during account management."""
+
+
+class InvalidAccountError(AccountsError):
+    """Errors raised when the account is invalid."""
+
+
+class AccountNotFoundError(AccountsError):
+    """Errors raised when the account is not found."""
+
+
+class AccountAlreadyExistsError(AccountsError):
+    """Errors raised when the account already exists."""
diff --git a/qiskit_ibm_runtime/accounts/management.py b/qiskit_ibm_runtime/accounts/management.py
index 8109b341..08063bb6 100644
--- a/qiskit_ibm_runtime/accounts/management.py
+++ b/qiskit_ibm_runtime/accounts/management.py
@@ -14,7 +14,7 @@
 
 import os
 from typing import Optional, Dict
-
+from .exceptions import AccountNotFoundError
 from .account import Account, AccountType
 from ..proxies import ProxyConfiguration
 from .storage import save_config, read_config, delete_config
@@ -42,13 +42,14 @@ class AccountManager:
         name: Optional[str] = _DEFAULT_ACCOUNT_NAME,
         proxies: Optional[ProxyConfiguration] = None,
         verify: Optional[bool] = None,
+        overwrite: Optional[bool] = False,
     ) -> None:
         """Save account on disk."""
-
         config_key = name or cls._get_default_account_name(auth)
         return save_config(
             filename=_DEFAULT_ACCOUNT_CONFIG_JSON_FILE,
             name=config_key,
+            overwrite=overwrite,
             config=Account(
                 token=token,
                 url=url,
@@ -122,14 +123,14 @@ class AccountManager:
             Account information.
 
         Raises:
-            ValueError: If the input value cannot be found on disk.
+            AccountNotFoundError: If the input value cannot be found on disk.
         """
         if name:
             saved_account = read_config(
                 filename=_DEFAULT_ACCOUNT_CONFIG_JSON_FILE, name=name
             )
             if not saved_account:
-                raise ValueError(
+                raise AccountNotFoundError(
                     f"Account with the name {name} does not exist on disk."
                 )
             return Account.from_saved_format(saved_account)
@@ -145,7 +146,7 @@ class AccountManager:
                 name=cls._get_default_account_name(auth),
             )
             if saved_account is None:
-                raise ValueError(f"No default {auth} account saved.")
+                raise AccountNotFoundError(f"No default {auth} account saved.")
             return Account.from_saved_format(saved_account)
 
         all_config = read_config(filename=_DEFAULT_ACCOUNT_CONFIG_JSON_FILE)
@@ -154,7 +155,7 @@ class AccountManager:
             if account_name in all_config:
                 return Account.from_saved_format(all_config[account_name])
 
-        return None
+        raise AccountNotFoundError("Unable to find account.")
 
     @classmethod
     def delete(
diff --git a/qiskit_ibm_runtime/accounts/storage.py b/qiskit_ibm_runtime/accounts/storage.py
index fa8cc560..1e59692e 100644
--- a/qiskit_ibm_runtime/accounts/storage.py
+++ b/qiskit_ibm_runtime/accounts/storage.py
@@ -16,15 +16,12 @@ import json
 import logging
 import os
 from typing import Optional, Dict
+from .exceptions import AccountAlreadyExistsError
 
 logger = logging.getLogger(__name__)
 
 
-def save_config(
-    filename: str,
-    name: str,
-    config: dict,
-) -> None:
+def save_config(filename: str, name: str, config: dict, overwrite: bool) -> None:
     """Save configuration data in a JSON file under the given name."""
     logger.debug("Save configuration data for '%s' in '%s'", name, filename)
     _ensure_file_exists(filename)
@@ -32,6 +29,12 @@ def save_config(
     with open(filename, mode="r") as json_in:
         data = json.load(json_in)
 
+    if data.get(name) and not overwrite:
+        raise AccountAlreadyExistsError(
+            f"Named account ({name}) already exists. "
+            f"Set overwrite=True to overwrite."
+        )
+
     with open(filename, mode="w") as json_out:
         data[name] = config
         json.dump(data, json_out, sort_keys=True, indent=4)
diff --git a/qiskit_ibm_runtime/ibm_runtime_service.py b/qiskit_ibm_runtime/ibm_runtime_service.py
index 83e58965..6776a6b3 100644
--- a/qiskit_ibm_runtime/ibm_runtime_service.py
+++ b/qiskit_ibm_runtime/ibm_runtime_service.py
@@ -25,7 +25,6 @@ from qiskit.providers.providerutils import filter_backends
 
 from qiskit_ibm_runtime import ibm_backend
 from .accounts import AccountManager, Account, AccountType
-from .accounts.exceptions import AccountsError
 from .proxies import ProxyConfiguration
 from .api.clients import AuthClient, VersionClient
 from .api.clients.runtime import RuntimeClient
@@ -250,8 +249,6 @@ class IBMRuntimeService:
 
         if account is None:
             account = AccountManager.get()
-            if account is None:
-                raise AccountsError("Unable to find account.")
 
         if instance:
             account.instance = instance
@@ -548,6 +545,7 @@ class IBMRuntimeService:
         name: Optional[str] = None,
         proxies: Optional[dict] = None,
         verify: Optional[bool] = None,
+        overwrite: Optional[bool] = False,
     ) -> None:
         """Save the account to disk for future use.
 
@@ -565,6 +563,7 @@ class IBMRuntimeService:
                 ``username_ntlm``, ``password_ntlm`` (username and password to enable NTLM user
                 authentication)
             verify: Verify the server's TLS certificate.
+            overwrite: ``True`` if the existing account is to be overwritten.
         """
 
         AccountManager.save(
@@ -575,6 +574,7 @@ class IBMRuntimeService:
             name=name,
             proxies=ProxyConfiguration(**proxies) if proxies else None,
             verify=verify,
+            overwrite=overwrite,
         )
 
     @staticmethod


Test patch:
diff --git a/test/test_account.py b/test/test_account.py
index cb99b81c..72e30211 100644
--- a/test/test_account.py
+++ b/test/test_account.py
@@ -19,7 +19,14 @@ import uuid
 from typing import Any
 from unittest import skipIf
 
-from qiskit_ibm_runtime.accounts import AccountManager, Account, management
+from qiskit_ibm_runtime.accounts import (
+    AccountManager,
+    Account,
+    management,
+    AccountAlreadyExistsError,
+    AccountNotFoundError,
+    InvalidAccountError,
+)
 from qiskit_ibm_runtime.proxies import ProxyConfiguration
 from qiskit_ibm_runtime.accounts.account import CLOUD_API_URL, LEGACY_API_URL
 from .ibm_test_case import IBMTestCase
@@ -59,7 +66,7 @@ class TestAccount(IBMTestCase):
     def test_invalid_auth(self):
         """Test invalid values for auth parameter."""
 
-        with self.assertRaises(ValueError) as err:
+        with self.assertRaises(InvalidAccountError) as err:
             invalid_auth: Any = "phantom"
             Account(
                 auth=invalid_auth, token=self.dummy_token, url=self.dummy_cloud_url
@@ -72,7 +79,7 @@ class TestAccount(IBMTestCase):
         invalid_tokens = [1, None, ""]
         for token in invalid_tokens:
             with self.subTest(token=token):
-                with self.assertRaises(ValueError) as err:
+                with self.assertRaises(InvalidAccountError) as err:
                     Account(
                         auth="cloud", token=token, url=self.dummy_cloud_url
                     ).validate()
@@ -86,7 +93,7 @@ class TestAccount(IBMTestCase):
         ]
         for params in subtests:
             with self.subTest(params=params):
-                with self.assertRaises(ValueError) as err:
+                with self.assertRaises(InvalidAccountError) as err:
                     Account(**params, token=self.dummy_token).validate()
                 self.assertIn("Invalid `url` value.", str(err.exception))
 
@@ -100,7 +107,7 @@ class TestAccount(IBMTestCase):
         ]
         for params in subtests:
             with self.subTest(params=params):
-                with self.assertRaises(ValueError) as err:
+                with self.assertRaises(InvalidAccountError) as err:
                     Account(
                         **params, token=self.dummy_token, url=self.dummy_cloud_url
                     ).validate()
@@ -137,6 +144,29 @@ class TestAccount(IBMTestCase):
 class TestAccountManager(IBMTestCase):
     """Tests for AccountManager class."""
 
+    @temporary_account_config_file(
+        contents={"conflict": _TEST_CLOUD_ACCOUNT.to_saved_format()}
+    )
+    def test_save_without_override(self):
+        """Test to override an existing account without setting overwrite=True."""
+        with self.assertRaises(AccountAlreadyExistsError):
+            AccountManager.save(
+                name="conflict",
+                token=_TEST_CLOUD_ACCOUNT.token,
+                url=_TEST_CLOUD_ACCOUNT.url,
+                instance=_TEST_CLOUD_ACCOUNT.instance,
+                auth="cloud",
+                overwrite=False,
+            )
+
+    @temporary_account_config_file(
+        contents={"conflict": _TEST_CLOUD_ACCOUNT.to_saved_format()}
+    )
+    def test_get_none(self):
+        """Test to get an account with an invalid name."""
+        with self.assertRaises(AccountNotFoundError):
+            AccountManager.get(name="bla")
+
     @temporary_account_config_file(contents={})
     @no_envs(["QISKIT_IBM_TOKEN"])
     def test_save_get(self):
@@ -172,6 +202,7 @@ class TestAccountManager(IBMTestCase):
                     proxies=account.proxies,
                     verify=account.verify,
                     name=name_save,
+                    overwrite=True,
                 )
                 self.assertEqual(account, AccountManager.get(name=name_get))
 
@@ -332,7 +363,7 @@ class TestEnableAccount(IBMTestCase):
         for url in urls:
             with self.subTest(url=url), no_envs(["QISKIT_IBM_TOKEN"]):
                 token = uuid.uuid4().hex
-                with self.assertRaises(ValueError) as err:
+                with self.assertRaises(InvalidAccountError) as err:
                     _ = FakeRuntimeService(auth="cloud", token=token, url=url)
                 self.assertIn("instance", str(err.exception))
 
@@ -439,7 +470,9 @@ class TestEnableAccount(IBMTestCase):
     def test_enable_account_bad_name(self):
         """Test initializing account by bad name."""
         name = "phantom"
-        with temporary_account_config_file() as _, self.assertRaises(ValueError) as err:
+        with temporary_account_config_file() as _, self.assertRaises(
+            AccountNotFoundError
+        ) as err:
             _ = FakeRuntimeService(name=name)
         self.assertIn(name, str(err.exception))