[OPIK-5496] [SDK] configure(force=True) still prompts interactively for workspace (#6033)

Author: yaricom

sdks/python/src/opik/configurator/configure.py

@@ -92,7 +92,7 @@ def _configure_cloud(self) -> None:
 
         # Update configuration if either API key or workspace has changed
         if update_config_with_api_key or update_config_with_workspace:
-            self._update_config()
+            self._update_config(save_to_file=True)
         else:
             self._update_config(save_to_file=False)
             _set_environment_variables_for_integrations(
@@ -152,7 +152,7 @@ def _configure_local(self) -> None:
 
             if use_url:
                 self.base_url = OPIK_BASE_URL_LOCAL
-                self._update_config()
+                self._update_config(save_to_file=True)
                 self._log_project_configuration_message()
                 return
 
@@ -162,7 +162,7 @@ def _configure_local(self) -> None:
                 "Opik URL is not specified - Please set your Opik instance URL using the environment variable OPIK_URL_OVERRIDE or provide it as an argument. For more details, refer to the documentation: https://www.comet.com/docs/opik/tracing/sdk_configuration."
             )
         self._ask_for_url()
-        self._update_config()
+        self._update_config(save_to_file=True)
         self._log_project_configuration_message()
 
     def _set_api_key(self) -> bool:
@@ -186,6 +186,16 @@ def _set_api_key(self) -> bool:
             self._try_set_url_from_api_key()
             config_file_needs_updating = True if self.force else False
 
+            if (
+                not config_file_needs_updating
+                and self.current_config.api_key is not None
+            ):
+                LOGGER.warning(
+                    "You already have an API key set in the configuration file. "
+                    "If you want to change it, please use the --force flag or force=True when calling the configure() method. "
+                    "Otherwise, the configuration file will not be updated but the session will use the new API key."
+                )
+
         elif self.force and self.api_key is None:
             self._ask_for_api_key()
             self._try_set_url_from_api_key()
@@ -294,7 +304,7 @@ def _set_workspace(self) -> bool:
         default_workspace = self._get_default_workspace()
         use_default_workspace = (
             True
-            if self.automatic_approvals
+            if self.automatic_approvals or self.force
             else ask_user_for_approval(
                 f'Do you want to use "{default_workspace}" workspace? (Y/n)'
             )
@@ -379,7 +389,7 @@ def _ask_for_workspace(self) -> None:
             "User does not have access to the workspaces provided."
         )
 
-    def _update_config(self, save_to_file: bool = True) -> None:
+    def _update_config(self, save_to_file: bool) -> None:
         """
         Save changes to the config file and update the current session configuration.
 

sdks/python/tests/library_integration/langchain/test_langgraph.py

@@ -84,10 +84,12 @@ def respond_to_greeting(state: State) -> Dict[str, Any]:
         input=initial_state,
         output=result,
         tags=["tag1", "tag2"],
-        metadata={
-            "a": "b",
-            "created_from": "langchain",
-        },
+        metadata=ANY_DICT.containing(
+            {
+                "a": "b",
+                "created_from": "langchain",
+            }
+        ),
         start_time=ANY_BUT_NONE,
         end_time=ANY_BUT_NONE,
         last_updated_at=ANY_BUT_NONE,
@@ -200,10 +202,12 @@ def invoke_graph_from_tracked_function(
                         input=initial_state,
                         output=result,
                         tags=["tag1", "tag2"],
-                        metadata={
-                            "a": "b",
-                            "created_from": "langchain",
-                        },
+                        metadata=ANY_DICT.containing(
+                            {
+                                "a": "b",
+                                "created_from": "langchain",
+                            }
+                        ),
                         start_time=ANY_BUT_NONE,
                         end_time=ANY_BUT_NONE,
                         spans=[
@@ -782,7 +786,9 @@ def respond_to_greeting(state: State) -> Dict[str, Any]:
                             "response": "Hello! How can I help you today?",
                         },
                         tags=["tag1", "tag2"],
-                        metadata={"a": "b", "created_from": "langchain"},
+                        metadata=ANY_DICT.containing(
+                            {"a": "b", "created_from": "langchain"}
+                        ),
                         type="general",
                         end_time=ANY_BUT_NONE,
                         project_name="langgraph-integration-test--distributed-headers",
@@ -933,7 +939,9 @@ def respond_to_greeting(state: State) -> Dict[str, Any]:
                             "response": "Hello! How can I help you today?",
                         },
                         tags=["tag1", "tag2"],
-                        metadata={"a": "b", "created_from": "langchain"},
+                        metadata=ANY_DICT.containing(
+                            {"a": "b", "created_from": "langchain"}
+                        ),
                         type="general",
                         end_time=ANY_BUT_NONE,
                         project_name="Default Project",
@@ -1072,7 +1080,7 @@ def respond_to_greeting(state: State) -> Dict[str, Any]:
                     "response": "Hello! How can I help you today?",
                 },
                 tags=["tag1", "tag2"],
-                metadata={"a": "b", "created_from": "langchain"},
+                metadata=ANY_DICT.containing({"a": "b", "created_from": "langchain"}),
                 type="general",
                 end_time=ANY_BUT_NONE,
                 project_name="Default Project",

sdks/python/tests/unit/configurator/test_configure.py

@@ -147,7 +147,7 @@ def test_update_config_success(self, mock_update_session_config, mock_opik_confi
         url = "http://example.com"
         workspace = "workspace1"
 
-        OpikConfigurator(api_key, workspace, url)._update_config()
+        OpikConfigurator(api_key, workspace, url)._update_config(save_to_file=True)
 
         # Ensure config object is created and saved
         mock_opik_config.assert_called_with(
@@ -179,7 +179,7 @@ def test_update_config_raises_exception(
         workspace = "workspace1"
 
         with pytest.raises(ConfigurationError, match="Failed to update configuration."):
-            OpikConfigurator(api_key, workspace, url)._update_config()
+            OpikConfigurator(api_key, workspace, url)._update_config(save_to_file=True)
 
         # Ensure save_to_file is not called due to the exception
         mock_update_session_config.assert_not_called()
@@ -201,7 +201,7 @@ def test_update_config_session_update_failure(
         workspace = "workspace1"
 
         with pytest.raises(ConfigurationError, match="Failed to update configuration."):
-            OpikConfigurator(api_key, workspace, url)._update_config()
+            OpikConfigurator(api_key, workspace, url)._update_config(save_to_file=True)
 
         # Ensure config object is created and saved
         mock_opik_config.assert_any_call(
@@ -567,6 +567,36 @@ def test_get_api_key_provided_key(self, mock_is_api_key_correct):
         assert configurator.api_key == "config_api_key"
         assert needs_update is True
 
+    @patch("opik.configurator.configure.LOGGER.warning")
+    @patch("opik.configurator.configure.opik.config.OpikConfig")
+    @patch(
+        "opik.configurator.configure.opik_rest_helpers.is_api_key_correct",
+        return_value=True,
+    )
+    def test_set_api_key__provided_api_key__another_key_already_set_in_config__not_forced__warning_is_shown(
+        self, mock_is_api_key_correct, mock_opik_config, mock_logger_warning
+    ):
+        """
+        Test that a warning is logged when an API key is provided, but one is already
+        set in the configuration file and force=False.
+        """
+        mock_config_instance = MagicMock()
+        mock_config_instance.api_key = "existing_api_key"
+        mock_opik_config.return_value = mock_config_instance
+
+        configurator = OpikConfigurator(
+            api_key="new_api_key", url=OPIK_BASE_URL_LOCAL, force=False
+        )
+        needs_update = configurator._set_api_key()
+
+        mock_logger_warning.assert_called_once_with(
+            "You already have an API key set in the configuration file. "
+            "If you want to change it, please use the --force flag or force=True when calling the configure() method. "
+            "Otherwise, the configuration file will not be updated but the session will use the new API key."
+        )
+        assert configurator.api_key == "new_api_key"
+        assert needs_update is False
+
     @patch(
         "opik.configurator.configure.opik_rest_helpers.is_api_key_correct",
         return_value=True,
@@ -785,6 +815,34 @@ def test_get_workspace_accept_default__automatic_approvals_enabled__no_approval_
         mock_get_default_workspace.assert_called_once_with()
         mock_ask_user_for_approval.assert_not_called()
 
+    @patch("opik.configurator.configure.opik.config.OpikConfig")
+    @patch(
+        "opik.configurator.configure.OpikConfigurator._get_default_workspace",
+        return_value="default_workspace",
+    )
+    @patch("opik.configurator.configure.ask_user_for_approval", return_value=True)
+    def test_get_workspace_accept_default__force_enabled__no_approval_questions_asked(
+        self, mock_ask_user_for_approval, mock_get_default_workspace, mock_opik_config
+    ):
+        """
+        Test that when force=True, the default workspace is used without asking for user approval.
+        """
+        current_config = OpikConfig(workspace=OPIK_WORKSPACE_DEFAULT_NAME)
+        mock_opik_config.return_value = current_config
+
+        configurator = OpikConfigurator(
+            workspace=None,
+            force=True,
+            api_key="valid_api_key",
+            automatic_approvals=False,
+        )
+        needs_update = configurator._set_workspace()
+
+        assert configurator.workspace == "default_workspace"
+        assert needs_update is True
+        mock_get_default_workspace.assert_called_once_with()
+        mock_ask_user_for_approval.assert_not_called()
+
     @patch("opik.configurator.configure.opik.config.OpikConfig")
     @patch(
         "opik.configurator.configure.OpikConfigurator._get_default_workspace",
@@ -1193,7 +1251,7 @@ def test_configure_local_uses_local_instance(
         mock_ask_user_for_approval.assert_called_once_with(
             f"Found local Opik instance on: {OPIK_BASE_URL_LOCAL}, do you want to use it? (Y/n)"
         )
-        mock_update_config.assert_called_once_with()
+        mock_update_config.assert_called_once_with(save_to_file=True)
 
         assert configurator.api_key is None
         assert configurator.base_url == OPIK_BASE_URL_LOCAL
@@ -1220,7 +1278,7 @@ def test_configure_local_uses_local_instance__automatic_approvals_enabled__no_ap
         configurator._configure_local()
 
         mock_ask_user_for_approval.assert_not_called()
-        mock_update_config.assert_called_once_with()
+        mock_update_config.assert_called_once_with(save_to_file=True)
 
         assert configurator.api_key is None
         assert configurator.base_url == OPIK_BASE_URL_LOCAL
@@ -1271,7 +1329,7 @@ def test_configure_local_uses_local_instance__non_interactive__automatic_approva
         configurator._configure_local()
 
         mock_ask_user_for_approval.assert_not_called()
-        mock_update_config.assert_called_once_with()
+        mock_update_config.assert_called_once_with(save_to_file=True)
 
         assert configurator.api_key is None
         assert configurator.base_url == OPIK_BASE_URL_LOCAL