resolved review comments

Author: Venkatakrishnan774

src/sap_cloud_sdk/outputmanagement/__init__.py

@@ -20,7 +20,7 @@
 from .models.form_configuration import FormConfiguration
 from .clients.email_client import EmailClient
 from .config.destination_credential_config import DestinationCredentialConfig
-from .constants import FileFormat, Channel, Status
+from .constants import FileFormat, Channel
 from .exceptions import (
     OutputManagementException,
     AuthenticationException,
@@ -54,7 +54,6 @@
     # Constants/Enums
     "FileFormat",
     "Channel",
-    "Status",
     # Exceptions
     "OutputManagementException",
     "AuthenticationException",

src/sap_cloud_sdk/outputmanagement/client.py

@@ -3,7 +3,9 @@
 import logging
 import requests
 from abc import ABC, abstractmethod
+from typing import Optional
 
+from sap_cloud_sdk.destination import Destination
 from .clients.output_requests_client import OutputRequestsClient
 from .clients.output_requests_client_impl import OutputRequestsClientImpl
 
@@ -35,8 +37,8 @@ class OutputManagementServiceDefaultClient(OutputManagementServiceClient):
     def __init__(
         self,
         base_url: str,
-        destination: any = None,
-        destination_instance: str = None,
+        destination: Optional[Destination] = None,
+        destination_instance: Optional[str] = None,
     ):
         """Initialize client.
 

src/sap_cloud_sdk/outputmanagement/client_provider.py

@@ -74,7 +74,7 @@ def build(self) -> OutputManagementServiceClientProvider:
         # Get the destination object - it handles authentication automatically
         http_destination = self._destination_credential_config.get_destination()
 
-        # Get the base URL from destinatiozxn
+        # Get the base URL from destination
         base_url = self._destination_credential_config.get_base_url()
         logger.info(f"Retrieved destination base URL: {base_url}")
 

src/sap_cloud_sdk/outputmanagement/clients/email_client.py

@@ -25,7 +25,7 @@ def create_output_request(
         to: List[str],
         business_document: Dict[str, Any],
         cc: Optional[List[str]] = None,
-        template_language: str = "en",
+        template_language: Optional[str] = "en",
         attachment_urls: Optional[List[str]] = None
     ) -> OutputRequest:
         """
@@ -121,8 +121,8 @@ def send_email(
         business_document: Dict[str, Any],
         destination_name: str,
         cc: Optional[List[str]] = None,
-        template_language: str = "en",
-        access_strategy: str = "PROVIDER_ONLY",
+        template_language: Optional[str] = "en",
+        access_strategy: Optional[str] = None,
         instance: Optional[str] = None,
         attachment_urls: Optional[List[str]] = None
     ) -> OutputResponse:
@@ -139,7 +139,7 @@ def send_email(
             destination_name: Name of the destination for authentication and endpoint
             cc: Optional list of CC email addresses
             template_language: ISO language code for email template (default: "en")
-            access_strategy: Destination access strategy - "PROVIDER_ONLY" or "SUBSCRIBER_ONLY" (default: "PROVIDER_ONLY")
+            access_strategy: Destination access strategy - "PROVIDER_ONLY" or "SUBSCRIBER_ONLY" (defaults to "PROVIDER_ONLY" if not specified)
             instance: Destination service instance name (defaults to "default" if not provided)
             attachment_urls: Optional list of DMS URLs for pre-generated attachments (default: None)
             

src/sap_cloud_sdk/outputmanagement/clients/output_requests_client_impl.py

@@ -1,18 +1,19 @@
 """Implementation of output requests client."""
 
 import logging
-from typing import Dict, Optional
 import os
-import json
 import uuid
+from typing import Dict, Optional
+
 import requests
+from sap_cloud_sdk.destination import Destination
 
 from .output_requests_client import OutputRequestsClient
+from ..constants import Constants
 from ..models.output_request import OutputRequest
 from ..models.output_response import (
     OutputResponse,
 )
-from ..constants import Constants
 from ..utils.request_validator import RequestValidator
 
 logger = logging.getLogger(__name__)
@@ -30,8 +31,8 @@ def __init__(
         self,
         http_session: requests.Session,
         base_url: str,
-        destination: any = None,
-        destination_instance: str = None,
+        destination: Optional[Destination] = None,
+        destination_instance: Optional[str] = None,
     ):
         """
         Constructs a new OutputRequestsClientImpl.
@@ -88,9 +89,8 @@ def send_output_request(self, output_request: OutputRequest) -> OutputResponse:
 
         try:
             request_body = output_request.model_dump(by_alias=True, exclude_none=True)
-            logger.info(f"Request body: {request_body}")
 
-            response = self._execute_request('POST', endpoint, json=request_body, headers=headers)
+            response = self._http_session.request('POST', endpoint, json=request_body, headers=headers)
             status_code = response.status_code
 
             logger.debug(f"Response status: {status_code}")
@@ -327,20 +327,6 @@ def _fetch_oauth_token_from_destination(self) -> Optional[str]:
 
         return None
 
-    def _execute_request(self, method: str, endpoint: str, **kwargs):
-        """Execute HTTP request using destination if available, otherwise use session.
-        
-        Args:
-            method: HTTP method (GET, POST, etc.)
-            endpoint: Full endpoint URL
-            **kwargs: Additional arguments to pass to the request
-            
-        Returns:
-            Response object
-        """
-        # Always use regular session - authentication is handled in _get_headers
-        return self._http_session.request(method, endpoint, **kwargs)
-    
     def _get_headers(self) -> Dict[str, str]:
         """Get request headers with authentication."""
         headers = {}

src/sap_cloud_sdk/outputmanagement/constants.py

@@ -10,8 +10,6 @@ class Constants:
     API_OUTPUT_CONTROL = "/api/output-control-api/v1/"
 
     # Headers
-    CONTENT_TYPE = "Content-Type"
-    APPLICATION_JSON = "application/json"
     AUTHORIZATION = "Authorization"
     BEARER = "Bearer"
     HEADER_CONTENT_TYPE = "Content-Type"
@@ -22,16 +20,6 @@ class Constants:
     CONTENT_TYPE_PDF = "application/pdf"
 
 
-class Status(Enum):
-    """Output request status."""
-
-    PENDING = "PENDING"
-    IN_PROGRESS = "IN_PROGRESS"
-    COMPLETED = "COMPLETED"
-    FAILED = "FAILED"
-    CANCELLED = "CANCELLED"
-
-
 class FileFormat(Enum):
     """Supported file formats."""
 

tests/outputmanagement/unit/test_basic.py

@@ -30,12 +30,10 @@ def test_constants_import(self):
             Constants,
             FileFormat,
             Channel,
-            Status,
         )
         assert Constants.API_OUTPUT_CONTROL is not None
         assert FileFormat.PDF is not None
         assert Channel.EMAIL is not None
-        assert Status.PENDING is not None
 
     def test_exceptions_import(self):
         """Test that exceptions can be imported."""

tests/outputmanagement/unit/test_constants.py

@@ -9,7 +9,6 @@
     Constants,
     FileFormat,
     Channel,
-    Status,
 )
 
 
@@ -28,15 +27,15 @@ def test_api_output_control_starts_with_slash(self):
 
     def test_header_constants_exist(self):
         """Test that header constants exist."""
-        assert Constants.CONTENT_TYPE is not None
-        assert Constants.APPLICATION_JSON is not None
+        assert Constants.HEADER_CONTENT_TYPE is not None
+        assert Constants.CONTENT_TYPE_JSON is not None
         assert Constants.AUTHORIZATION is not None
         assert Constants.BEARER is not None
 
     def test_header_constants_are_strings(self):
         """Test that header constants are strings."""
-        assert isinstance(Constants.CONTENT_TYPE, str)
-        assert isinstance(Constants.APPLICATION_JSON, str)
+        assert isinstance(Constants.HEADER_CONTENT_TYPE, str)
+        assert isinstance(Constants.CONTENT_TYPE_JSON, str)
         assert isinstance(Constants.AUTHORIZATION, str)
         assert isinstance(Constants.BEARER, str)
 
@@ -58,11 +57,3 @@ def test_channel_enum(self):
         assert Channel.INTERNAL_EMAIL.value == "INTERNAL_EMAIL"
         assert Channel.DIRECT_SHARE.value == "DIRECT_SHARE"
         assert Channel.FORM.value == "FORM"
-
-    def test_status_enum(self):
-        """Test Status enum."""
-        assert Status.PENDING.value == "PENDING"
-        assert Status.IN_PROGRESS.value == "IN_PROGRESS"
-        assert Status.COMPLETED.value == "COMPLETED"
-        assert Status.FAILED.value == "FAILED"
-        assert Status.CANCELLED.value == "CANCELLED"