diff --git a/codecarbon/cli/main.py b/codecarbon/cli/main.py index c10b32338..7eddea377 100644 --- a/codecarbon/cli/main.py +++ b/codecarbon/cli/main.py @@ -116,7 +116,11 @@ def api_get(): api_endpoint = get_api_endpoint() api = ApiClient(endpoint_url=api_endpoint) api.set_access_token(get_access_token()) - organizations = api.get_list_organizations() + try: + organizations = api.get_list_organizations() + except Exception as e: + print(f"[yellow]API request failed[/yellow]. (error: {e})") + raise typer.Exit(1) print(organizations) @@ -130,7 +134,11 @@ def login(): api = ApiClient(endpoint_url=api_endpoint) access_token = get_access_token() api.set_access_token(access_token) - api.check_auth() + try: + api.check_auth() + except Exception as e: + print(f"[yellow]Authentication check failed[/yellow]. (error: {e})") + raise typer.Exit(1) def get_api_key(project_id: str): @@ -212,7 +220,14 @@ def config(): overwrite_local_config("api_endpoint", api_endpoint, path=file_path) api = ApiClient(endpoint_url=api_endpoint) api.set_access_token(get_access_token()) - organizations = api.get_list_organizations() + try: + organizations = api.get_list_organizations() + except Exception as e: + print( + f"[yellow]Could not list organizations from API[/yellow]. " + f"Please check your login and API endpoint. (error: {e})" + ) + return org = questionary_prompt( "Pick existing organization from list or Create new organization ?", [org["name"] for org in organizations] + ["Create New Organization"], @@ -230,9 +245,6 @@ def config(): description=org_description, ) organization = api.create_organization(organization=organization_create) - if organization is None: - print("Error creating organization") - return print(f"Created organization : {organization}") else: organization = [orga for orga in organizations if orga["name"] == org][0] @@ -240,7 +252,7 @@ def config(): overwrite_local_config("organization_id", org_id, path=file_path) projects = api.list_projects_from_organization(org_id) - project_names = [project["name"] for project in projects] if projects else [] + project_names = [project["name"] for project in projects] project = questionary_prompt( "Pick existing project from list or Create new project ?", project_names + ["Create New Project"], @@ -264,9 +276,7 @@ def config(): overwrite_local_config("project_id", project_id, path=file_path) experiments = api.list_experiments_from_project(project_id) - experiments_names = ( - [experiment["name"] for experiment in experiments] if experiments else [] - ) + experiments_names = [experiment["name"] for experiment in experiments] experiment = questionary_prompt( "Pick existing experiment from list or Create new experiment ?", diff --git a/codecarbon/core/api_client.py b/codecarbon/core/api_client.py index 58f4932cb..3d031cfc5 100644 --- a/codecarbon/core/api_client.py +++ b/codecarbon/core/api_client.py @@ -85,8 +85,7 @@ def check_auth(self): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() def get_list_organizations(self): @@ -97,8 +96,7 @@ def get_list_organizations(self): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() def check_organization_exists(self, organization_name: str): @@ -106,8 +104,6 @@ def check_organization_exists(self, organization_name: str): Check if an organization exists """ organizations = self.get_list_organizations() - if organizations is None: - return False for organization in organizations: if organization["name"] == organization_name: return organization @@ -128,8 +124,7 @@ def create_organization(self, organization: OrganizationCreate): headers = self._get_headers() r = requests.post(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 201: - self._log_error(url, payload, r) - return None + self._raise_api_error(url, payload, r) return r.json() def get_organization(self, organization_id): @@ -140,8 +135,7 @@ def get_organization(self, organization_id): url = self.url + "/organizations/" + organization_id r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() def update_organization(self, organization: OrganizationCreate): @@ -153,8 +147,7 @@ def update_organization(self, organization: OrganizationCreate): url = self.url + "/organizations/" + organization.id r = requests.patch(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, payload, r) - return None + self._raise_api_error(url, payload, r) return r.json() def list_projects_from_organization(self, organization_id): @@ -165,8 +158,7 @@ def list_projects_from_organization(self, organization_id): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() def create_project(self, project: ProjectCreate): @@ -178,8 +170,7 @@ def create_project(self, project: ProjectCreate): headers = self._get_headers() r = requests.post(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 201: - self._log_error(url, payload, r) - return None + self._raise_api_error(url, payload, r) return r.json() def get_project(self, project_id): @@ -190,8 +181,7 @@ def get_project(self, project_id): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() def add_emission(self, carbon_emission: dict): @@ -236,12 +226,13 @@ def add_emission(self, carbon_emission: dict): headers = self._get_headers() r = requests.post(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 201: - self._log_error(url, payload, r) - return False + self._raise_api_error(url, payload, r) logger.debug(f"ApiClient - Successful upload emission {payload} to {url}") + except requests.exceptions.HTTPError: + raise except Exception as e: logger.error(e, exc_info=True) - return False + raise return True def _create_run(self, experiment_id: str): @@ -278,8 +269,7 @@ def _create_run(self, experiment_id: str): headers = self._get_headers() r = requests.post(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 201: - self._log_error(url, payload, r) - return None + self._raise_api_error(url, payload, r) self.run_id = r.json()["id"] logger.info( "ApiClient Successfully registered your run on the API.\n\n" @@ -292,8 +282,12 @@ def _create_run(self, experiment_id: str): f"Failed to connect to API, please check the configuration. {e}", exc_info=False, ) + raise + except requests.exceptions.HTTPError: + raise except Exception as e: logger.error(e, exc_info=True) + raise def list_experiments_from_project(self, project_id: str): """ @@ -303,8 +297,7 @@ def list_experiments_from_project(self, project_id: str): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return [] + self._raise_api_error(url, {}, r) return r.json() def set_experiment(self, experiment_id: str): @@ -323,8 +316,7 @@ def add_experiment(self, experiment: ExperimentCreate): headers = self._get_headers() r = requests.post(url=url, json=payload, timeout=2, headers=headers) if r.status_code != 201: - self._log_error(url, payload, r) - return None + self._raise_api_error(url, payload, r) return r.json() def get_experiment(self, experiment_id): @@ -335,11 +327,10 @@ def get_experiment(self, experiment_id): headers = self._get_headers() r = requests.get(url=url, timeout=2, headers=headers) if r.status_code != 200: - self._log_error(url, {}, r) - return None + self._raise_api_error(url, {}, r) return r.json() - def _log_error(self, url, payload, response): + def _raise_api_error(self, url, payload, response): if len(payload) > 0: logger.error( f"ApiClient Error when calling the API on {url} with : {json.dumps(payload)}" @@ -349,6 +340,11 @@ def _log_error(self, url, payload, response): logger.error( f"ApiClient API return http code {response.status_code} and answer : {response.text}" ) + response.raise_for_status() + # 2xx/3xx that still isn't what the caller expected + raise requests.exceptions.HTTPError( + f"Unexpected status {response.status_code} from {url}", response=response + ) def close_experiment(self): """ diff --git a/codecarbon/emissions_tracker.py b/codecarbon/emissions_tracker.py index 270723466..3ea26b072 100644 --- a/codecarbon/emissions_tracker.py +++ b/codecarbon/emissions_tracker.py @@ -637,14 +637,23 @@ def _init_output_methods(self, *, api_key: str = None): self._output_handlers.append(HTTPOutput(self._emissions_endpoint)) if OutputMethod.API in methods: - cc_api__out = CodeCarbonAPIOutput( - endpoint_url=self._api_endpoint, - experiment_id=self._experiment_id, - api_key=api_key, - conf=self._conf, - ) - self.run_id = cc_api__out.run_id - self._output_handlers.append(cc_api__out) + try: + cc_api__out = CodeCarbonAPIOutput( + endpoint_url=self._api_endpoint, + experiment_id=self._experiment_id, + api_key=api_key, + conf=self._conf, + ) + self.run_id = cc_api__out.run_id + self._output_handlers.append(cc_api__out) + except Exception as e: + logger.error( + "Failed to initialize Code Carbon API output; " + "continuing without API reporting. %s", + e, + exc_info=True, + ) + self.run_id = uuid.uuid4() else: self.run_id = uuid.uuid4() diff --git a/tests/cli/test_cli_main.py b/tests/cli/test_cli_main.py index 84f42493d..b0c3ec593 100644 --- a/tests/cli/test_cli_main.py +++ b/tests/cli/test_cli_main.py @@ -3,6 +3,7 @@ from types import SimpleNamespace import pytest +import requests import typer from typer.testing import CliRunner @@ -43,6 +44,21 @@ def test_api_get_calls_api_and_prints(monkeypatch): assert "fake-org" in result.output +def test_api_get_prints_friendly_error_on_api_failure(monkeypatch): + class FailingApiClient(FakeApiClient): + def get_list_organizations(self): + raise requests.exceptions.HTTPError("401 Unauthorized") + + runner = CliRunner() + monkeypatch.setattr("codecarbon.core.api_client.ApiClient", FailingApiClient) + monkeypatch.setattr("codecarbon.cli.auth.get_access_token", fake_get_access_token) + + result = runner.invoke(cli_main.codecarbon, ["test-api"]) + assert result.exit_code == 1 + assert "API request failed" in result.output + assert "401 Unauthorized" in result.output + + def test_api_get_uses_get_api_endpoint(monkeypatch): call_info = {} @@ -185,6 +201,31 @@ def check_auth(self): assert calls["endpoint_url"] == "https://custom-login.codecarbon.io" +def test_login_prints_friendly_error_on_auth_failure(monkeypatch): + class FailingApiClient: + def __init__(self, endpoint_url=None): + pass + + def set_access_token(self, token): + pass + + def check_auth(self): + raise requests.exceptions.HTTPError("403 Forbidden") + + monkeypatch.setattr("codecarbon.core.api_client.ApiClient", FailingApiClient) + monkeypatch.setattr("codecarbon.cli.auth.authorize", lambda: None) + monkeypatch.setattr( + cli_main, "get_api_endpoint", lambda: "https://api.codecarbon.io" + ) + monkeypatch.setattr("codecarbon.cli.auth.get_access_token", lambda: "bad-token") + + runner = CliRunner() + result = runner.invoke(cli_main.codecarbon, ["login"]) + assert result.exit_code == 1 + assert "Authentication check failed" in result.output + assert "403 Forbidden" in result.output + + def test_get_api_key_uses_bearer_token(monkeypatch): captured = {} diff --git a/tests/test_api_call.py b/tests/test_api_call.py index 39822ece7..a5178f7b3 100644 --- a/tests/test_api_call.py +++ b/tests/test_api_call.py @@ -2,6 +2,7 @@ import unittest from uuid import uuid4 +import requests import requests_mock from codecarbon.core.api_client import ApiClient @@ -137,7 +138,7 @@ def test_call_api(self): assert payload["ram_utilization_percent"] == 56.5 assert payload["wue"] == 0.8 - def test_check_auth_returns_none_on_error(self): + def test_check_auth_raises_on_error(self): with requests_mock.Mocker() as m: m.get("http://test.com/auth/check", text="bad", status_code=401) api = ApiClient( @@ -146,9 +147,10 @@ def test_check_auth_returns_none_on_error(self): create_run_automatically=False, ) - self.assertIsNone(api.check_auth()) + with self.assertRaises(requests.exceptions.HTTPError): + api.check_auth() - def test_check_organization_exists_returns_false_when_list_fails(self): + def test_check_organization_exists_raises_when_list_fails(self): with requests_mock.Mocker() as m: m.get("http://test.com/organizations", text="bad", status_code=500) api = ApiClient( @@ -156,7 +158,8 @@ def test_check_organization_exists_returns_false_when_list_fails(self): create_run_automatically=False, ) - self.assertFalse(api.check_organization_exists("missing")) + with self.assertRaises(requests.exceptions.HTTPError): + api.check_organization_exists("missing") def test_create_organization_skips_when_name_exists(self): organization = OrganizationCreate(name="existing", description="desc") @@ -225,7 +228,7 @@ def test_add_emission_skips_short_duration(self): ) ) - def test_add_emission_returns_false_on_unsuccessful_post(self): + def test_add_emission_raises_on_unsuccessful_post(self): with requests_mock.Mocker() as m: m.post("http://test.com/emissions", text="bad", status_code=500) api = ApiClient( @@ -236,7 +239,7 @@ def test_add_emission_returns_false_on_unsuccessful_post(self): ) api.run_id = "run-1" - self.assertFalse( + with self.assertRaises(requests.exceptions.HTTPError): api.add_emission( { "duration": 2, @@ -251,9 +254,8 @@ def test_add_emission_returns_false_on_unsuccessful_post(self): "energy_consumed": 0.2, } ) - ) - def test_create_run_returns_none_on_unsuccessful_status(self): + def test_create_run_raises_on_unsuccessful_status(self): with requests_mock.Mocker() as m: m.post("http://test.com/runs", text="bad", status_code=400) api = ApiClient( @@ -264,10 +266,55 @@ def test_create_run_returns_none_on_unsuccessful_status(self): create_run_automatically=False, ) - self.assertIsNone(api._create_run("experiment_id")) + with self.assertRaises(requests.exceptions.HTTPError): + api._create_run("experiment_id") self.assertIsNone(api.run_id) - def test_list_experiments_from_project_returns_empty_list_on_error(self): + def test_create_run_raises_on_unexpected_2xx_status(self): + with requests_mock.Mocker() as m: + m.post("http://test.com/runs", json={}, status_code=200) + api = ApiClient( + endpoint_url="http://test.com", + experiment_id="experiment_id", + api_key="Toto", + conf=conf, + create_run_automatically=False, + ) + + with self.assertRaises(requests.exceptions.HTTPError) as ctx: + api._create_run("experiment_id") + self.assertIn("Unexpected status 200", str(ctx.exception)) + self.assertIsNone(api.run_id) + + def test_add_emission_raises_on_unexpected_2xx_status(self): + with requests_mock.Mocker() as m: + m.post("http://test.com/emissions", json={}, status_code=200) + api = ApiClient( + endpoint_url="http://test.com", + experiment_id="exp-1", + conf=conf, + create_run_automatically=False, + ) + api.run_id = "run-1" + + with self.assertRaises(requests.exceptions.HTTPError) as ctx: + api.add_emission( + { + "duration": 2, + "emissions": 1.0, + "emissions_rate": 1.0, + "cpu_power": 1.0, + "gpu_power": 0.0, + "ram_power": 0.5, + "cpu_energy": 0.1, + "gpu_energy": 0.0, + "ram_energy": 0.1, + "energy_consumed": 0.2, + } + ) + self.assertIn("Unexpected status 200", str(ctx.exception)) + + def test_list_experiments_from_project_raises_on_error(self): with requests_mock.Mocker() as m: m.get( "http://test.com/projects/proj-1/experiments", @@ -279,7 +326,8 @@ def test_list_experiments_from_project_returns_empty_list_on_error(self): create_run_automatically=False, ) - self.assertEqual(api.list_experiments_from_project("proj-1"), []) + with self.assertRaises(requests.exceptions.HTTPError): + api.list_experiments_from_project("proj-1") def test_set_experiment_updates_value(self): api = ApiClient(endpoint_url="http://test.com", create_run_automatically=False) @@ -288,7 +336,7 @@ def test_set_experiment_updates_value(self): self.assertEqual(api.experiment_id, "exp-2") - def test_add_experiment_returns_none_on_error(self): + def test_add_experiment_raises_on_error(self): experiment = ExperimentCreate( timestamp="2024-01-01T00:00:00+00:00", name="exp", @@ -303,9 +351,10 @@ def test_add_experiment_returns_none_on_error(self): create_run_automatically=False, ) - self.assertIsNone(api.add_experiment(experiment)) + with self.assertRaises(requests.exceptions.HTTPError): + api.add_experiment(experiment) - def test_get_experiment_returns_none_on_error(self): + def test_get_experiment_raises_on_error(self): with requests_mock.Mocker() as m: m.get("http://test.com/experiments/exp-1", text="bad", status_code=404) api = ApiClient( @@ -313,4 +362,19 @@ def test_get_experiment_returns_none_on_error(self): create_run_automatically=False, ) - self.assertIsNone(api.get_experiment("exp-1")) + with self.assertRaises(requests.exceptions.HTTPError): + api.get_experiment("exp-1") + + def test_create_run_raises_on_connection_error(self): + with requests_mock.Mocker() as m: + m.post( + "http://test.com/runs", + exc=requests.exceptions.ConnectionError("API unreachable"), + ) + with self.assertRaises(requests.exceptions.ConnectionError): + ApiClient( + experiment_id="experiment_id", + endpoint_url="http://test.com", + api_key="Toto", + conf=conf, + ) diff --git a/tests/test_emissions_tracker.py b/tests/test_emissions_tracker.py index 8ab12e5d8..584f00321 100644 --- a/tests/test_emissions_tracker.py +++ b/tests/test_emissions_tracker.py @@ -425,6 +425,37 @@ def test_output_methods_overrides_save_to_flags( ) ) + def test_api_output_init_failure_does_not_break_tracker( + self, + mock_cli_setup, + mock_log_values, + mocked_get_gpu_details, + mocked_env_cloud_details, + mocked_get_gpu_utilization_list, + mocked_is_gpu_details_available, + mocked_is_nvidia_system, + ): + with mock.patch( + "codecarbon.output_methods.http.CodeCarbonAPIOutput", + side_effect=requests.exceptions.HTTPError("API unavailable"), + ) as mock_api_output: + tracker = EmissionsTracker( + output_dir=self.temp_path, + output_handlers=[], + output_methods=[OutputMethod.CSV, OutputMethod.API], + api_key="test-key", + experiment_id="exp-1", + ) + + mock_api_output.assert_called_once() + self.assertIsNotNone(tracker.run_id) + self.assertFalse( + any( + isinstance(handler, CodeCarbonAPIOutput) + for handler in tracker._output_handlers + ) + ) + def test_output_methods_parsed_from_config_string( self, mock_cli_setup,