fix: the api key credentials weren't registering correctly #2
Conversation
🤖 Augment PR SummarySummary: This PR fixes API-key credential registration for SDK providers so default credentials are created and surfaced consistently. Changes:
Technical Notes: The credentials-store patch respects module mocking via 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| # Check if credential already exists to avoid duplicates | ||
| if not any(c.id == credential.id for c in cls._default_credentials): | ||
| cls._default_credentials.append(credential) | ||
| cls._lock.acquire() |
There was a problem hiding this comment.
register_api_key() calls cls._lock.acquire() but never releases it, which can deadlock subsequent registry operations (including get_all_credentials() / register_provider()). Consider using with cls._lock: or ensuring release() in a finally block.
🤖 Was this useful? React with 👍 or 👎
| # Combine credentials, avoiding duplicates by ID | ||
| existing_ids={c.id for c in original_creds} | ||
| for cred in sdk_creds: | ||
| if cred.id in existing_ids:original_creds.append(cred) |
There was a problem hiding this comment.
This duplicate-avoidance logic looks inverted: if cred.id in existing_ids: original_creds.append(cred) will skip new SDK creds and/or append duplicates of existing ones. Consider only appending when the ID is missing (and updating existing_ids accordingly) so SDK credentials actually get included once.
🤖 Was this useful? React with 👍 or 👎
Benchmark PR from qodo-benchmark#10