-
Notifications
You must be signed in to change notification settings - Fork 275
feat(python): add import_paths config for custom code integration #11805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(python): add import_paths config for custom code integration #11805
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: [email protected] <[email protected]>
Replace sentry-specific implementation with generalized require-paths config option. This allows users to auto-load custom code files when the SDK is imported. Changes: - Add require_paths config option to SDKCustomConfig (array of strings) - Add import hook at end of main __init__.py to auto-load specified files - Remove sentry-specific code (SentryIntegrationConfig, sentry_integration.py template) - Update test fixture from sentry-integration to require-paths - Update versions.yml with changelog entry Co-Authored-By: [email protected] <[email protected]>
…69464083-sentry-integration
| if generator_config.custom_config is not None: | ||
| # Support both hyphenated and underscored versions | ||
| require_paths = generator_config.custom_config.get( | ||
| "require_paths", generator_config.custom_config.get("require-paths") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove all the kebab-case support. We don't do it elsewhere in the python generator so I think it's just confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think require_paths is the right term here? It was a bit confusing to me that we import from a require_path (require IMO is not a python term) and then idk how package_name relates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I used require_paths to match the Ruby SDK's requirePaths config option for consistency across generators (as requested in the original task to duplicate Ruby PR #11264).
However, you're right that "require" is more of a Ruby term. A more Pythonic name would be import_paths or auto_import_paths.
Regarding package_name - it's used in the generated import statement: import_module(f".{_path}", "seed") where "seed" is the package name. The .{_path} is a relative import within that package.
Would you prefer I rename this to import_paths for better Python semantics? Happy to make that change if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, import_paths is PERFECT!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! I've renamed require_paths to import_paths throughout the codebase:
- Updated all Python generator files
- Renamed the fixture folder from
require-pathstoimport-paths - Updated the changelog to use
import_paths
The config option is now import_paths (snake_case, Pythonic naming).
Description
Refs: Requested by @aditya-arolkar-swe
Adds a generalized
import_pathsconfig option to the Python SDK generator that auto-loads user-defined files when the SDK is imported. This follows the same pattern as the Ruby SDK generator (PR #11264), but uses Pythonic naming (import_pathsinstead ofrequirePaths).Link to Devin run: https://app.devin.ai/sessions/9e243bc5375e42018712dffa54f7546f
Changes Made
import_pathsconfig option toSDKCustomConfig(array of strings)_write_import_paths_hook()function inmodule_manager.pythat writes import hooks at the end of root__init__.pyProjectclass to passimport_pathsandpackage_nametoModuleManagerabstract_generator.pyto readimport_pathsconfig and pass to Projectget_import_paths()method toCoreUtilitiesexhaustive:import-pathsto validate the featureUsage
Generator config:
Generated code in root
__init__.py:Example user-defined
sentry_integration.py:Human Review Checklist
__init__.pyhas the import hook at the end - checkseed/python-sdk/exhaustive/import-paths/src/seed/__init__.pylines 64-70import_moduleimport is available (already imported for lazy imports, conditionally added otherwise at line 236 ofmodule_manager.py)package_pathconfig correctlyimport_paths) not kebab-caseTesting
exhaustive:import-pathspasses--skip-scripts)Updates Since Last Revision
require_pathstoimport_pathsfor more Pythonic naming (per reviewer feedback)require-paths) - only snake_case (import_paths) is supportedrequire-pathstoimport-pathsimport_pathsterminology