Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2f7102897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def __call__(cls, *args: Any, **kwargs: Any): | ||
| version = kwargs.pop("version", None) | ||
|
|
||
| # Only dispatch for the public front class itself. | ||
| if cls.__dict__.get("_DISPATCH", False): | ||
| # Backwards-compatible default: if no version is provided, return the | ||
| # oldest supported implementation. | ||
| v_raw = version if version is not None else getattr(cls, "_DEFAULT_VERSION", None) | ||
| if v_raw is None: | ||
| return super().__call__(*args, **kwargs) | ||
|
|
||
| v_raw = getattr(v_raw, "value", v_raw) | ||
| v = str(v_raw) | ||
| aliases: dict[str, str] = getattr(cls, "_VERSION_ALIASES", {}) | ||
| v_norm = aliases.get(v, v) | ||
|
|
||
| impl_map: dict[str, type] = getattr(cls, "_VERSION_MAP", {}) | ||
| impl = impl_map.get(v_norm) | ||
| if impl is not None: | ||
| return impl(*args, **kwargs) |
There was a problem hiding this comment.
The metaclass strips version from kwargs and instantiates the implementation class without passing it through, so Client.__init__ receives version=None. For versioned APIs that previously stored the requested version (e.g., CatalogItems/ListingsItems via self.version), CatalogItems(version="2022-04-01").version now becomes None. This is a behavioral regression for callers who introspect client.version or downstream helpers that rely on it. Consider forwarding the normalized version into the implementation constructor (or explicitly setting self.version on the concrete class) to preserve prior behavior.
Useful? React with 👍 / 👎.
| def _ensure_models_repo() -> Path: | ||
| base = Path(tempfile.gettempdir()) | ||
| repo_root = base / MODELS_REPO_DIRNAME | ||
| models_dir = repo_root / "models" | ||
| if models_dir.exists(): | ||
| return models_dir | ||
|
|
||
| subprocess.run( | ||
| ["git", "clone", "--depth", "1", MODELS_REPO_URL, str(repo_root)], | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
The test clones the models repository at HEAD on every new environment, which makes the suite nondeterministic and dependent on network availability. If the upstream repo changes (or CI runs offline), these tests can fail without any changes to this codebase. Pinning to a specific commit or vendoring the models (or caching them in-repo) would make the test stable.
Useful? React with 👍 / 👎.
|
@codex detect breaking changes |
|
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2f7102897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| from sp_api.base import Client | ||
|
|
||
| from .finances_2024_06_01 import FinancesV20240601 |
There was a problem hiding this comment.
This commit imports FinancesV20240601 from sp_api/api/finances/finances_2024_06_01.py, but that module is not present in the f510 tree (checked the commit’s file list via git ls-tree -r f510... and it’s absent). As a result, importing sp_api.api.finances (or the top-level sp_api.api which re-exports it) will raise an ImportError at runtime. Either add the missing module to the commit or remove the import/version entry.
Useful? React with 👍 / 👎.
automate adding new clients, improve documentation
1 participant
No description provided.