Skip to content

Conversation

@kadirpekel
Copy link
Collaborator

No description provided.

Copy link
Contributor

@ahmetgunduz ahmetgunduz left a comment

Choose a reason for hiding this comment

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

This is a great design patterns (inspector framework, multi-instance support, resource mixins). However, there are some blocking issues before merge:

🔴 Critical Issue Fixed

  • Python 3.10 compatibility bug: NotRequired was imported from typing instead of typing_extensions, breaking all imports on Python 3.10. ✅ Fixed in aixplain/v2/resource.py

❌ Blocking Issues Remaining

  1. 14/29 unit tests failing - Tests need updates for new API structure:

    • test_resource.py - BaseResource API changed (no obj parameter)
    • test_apikey_multi_instance.py - Mocks need updating
    • test_core.py - Partially fixed (Pipeline → Tool) but more issues remain
  2. Documentation outdated - docs/api-reference/python/api_sidebar.js still references 9 deleted modules (pipeline, api_key, benchmark, etc.)

  3. Functional tests - Need verification that they pass

Checklist

  • Code follows project style guidelines
  • No linter errors in production code
  • All unit tests passing (14/29 failing)
  • All functional tests passing (not verified)
  • Python 3.10 compatibility fixed
  • Documentation updated
  • [ ]Test Coverage 90+%

@ahmetgunduz
Copy link
Contributor

@kadirpekel After resolving the above, I would like to do a detailed design review in a second round.

@kadirpekel
Copy link
Collaborator Author

I am working on it

@kadirpekel
Copy link
Collaborator Author

@ahmetgunduz @tim-nelson addressed all the issues apparently. all functional and unit tests passing, python 10 compatibility issue fixed, updated documentation

@ahmetgunduz
Copy link
Contributor

@kadirpekel is this for a review?

@kadirpekel kadirpekel merged commit d0fc679 into development Nov 14, 2025
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.

3 participants