Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

Fixed inconsistent base_url handling in openai patch.py:

  1. Added get_llm_url to async_embeddings_create
  2. Added missing provider checks (GROQ, DEEPSEEK) to async_chat_completions_create
  3. Added provider checks to embeddings_create and async_embeddings_create
  4. Added test for embeddings base_url handling
  5. Added debug logging to help diagnose provider detection issues

Changes have been tested and verified working with both OpenAI and Azure providers.

Link to Devin run: https://preview.devin.ai/devin/c6cc20c608ec49afaca906f05d2eab41

@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Add "(aside)" to your comment to have me ignore it.

lb7OIbyQkMNb/1X/+jSA0GPOEnWXk5wtxWunQXV5p5u/ANXWr5Fge0catsJrHC1hKrx/ef6v3lfB
iLsQDjRukLDp7T/5d8oOLj2c9oO39YcMactvsP2e+myuW8+Fp7YM8DHkF4+iIn9Lm76n2t6s9Ymc
biG89x8ZCeaHrZbPWljypn/+9JPXc3MuYX5CLjW/0dHrDn2SgL9/pwL+819//fW/ficMmvb+qLeD
AeNjHv/930cF/p3e039zHP/nGAIZ0uLx9z//dQLh72/fNt/xf4/t+/EZ/v7nL/7PUYO/x3ZM6//n
Copy link
Collaborator

@alizenhom alizenhom Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some sensitive data is present here,

can you change inside conftest.py the vcr config to

@pytest.fixture(scope="module")
def vcr_config():
    return {
        "filter_headers": [
            ("cookie", "test_cookie"),
            ("authorization", "Bearer test_openai_api_key"),
            ("openai-organization", "test_openai_org_id"),
            ("openai-project", "test_openai_project_id"),
        ],
        "decode_compressed_response": True,
        "before_record_response": scrub_response_headers,
    }

def scrub_response_headers(response):
    """
    This scrubs sensitive response headers. Note they are case-sensitive!
    """
    response["headers"]["openai-organization"] = "test_openai_org_id"
    response["headers"]["Set-Cookie"] = "test_set_cookie"
    return response

Copy link
Collaborator

@alizenhom alizenhom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly adjust the test recording to not leak any sensitive data



@pytest.fixture(scope="module")
def vcr_config():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change the vcr_config method to

@pytest.fixture(scope="module")
def vcr_config():
    return {
        "filter_headers": [
            ("cookie", "test_cookie"),
            ("authorization", "Bearer test_openai_api_key"),
            ("openai-organization", "test_openai_org_id"),
            ("openai-project", "test_openai_project_id"),
        ],
        "decode_compressed_response": True,
        "before_record_response": scrub_response_headers,
    }

and scrub_response_headers is

def scrub_response_headers(response):
    """
    This scrubs sensitive response headers. Note they are case-sensitive!
    """
    response["headers"]["openai-organization"] = "test_openai_org_id"
    response["headers"]["Set-Cookie"] = "test_set_cookie"
    return response

@alizenhom alizenhom changed the base branch from main to development December 17, 2024 16:00
@alizenhom alizenhom force-pushed the devin/1733790643-fix-base-url-handling branch 2 times, most recently from 28c372a to d8a48f0 Compare December 17, 2024 16:44
@alizenhom alizenhom merged commit 1bc18e7 into development Dec 17, 2024
8 checks passed
@alizenhom alizenhom deleted the devin/1733790643-fix-base-url-handling branch December 17, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants