-
Notifications
You must be signed in to change notification settings - Fork 13
Defer code generator validation until transform submission #644
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
Defer code generator validation until transform submission #644
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #644 +/- ##
==========================================
+ Coverage 97.19% 98.04% +0.84%
==========================================
Files 29 29
Lines 1959 2043 +84
==========================================
+ Hits 1904 2003 +99
+ Misses 55 40 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Marked as draft until I come back from vacation. If urgent and someone else wants to tackle it, then go for it! |
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.
Pull Request Overview
This PR defers ServiceX code generator validation until transform submission to optimize performance and avoid unnecessary network calls for cached queries.
- Lazy loading of code generators with caching to avoid network calls when ServiceX client is instantiated
- Code generator validation moved from client instantiation to transform submission time
- Enhanced error messages that list available code generators when an unsupported one is requested
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| servicex/servicex_client.py | Implements lazy loading of code generators with caching and removes upfront validation |
| servicex/query_core.py | Adds async code generator validation with helpful error messages during transform submission |
| tests/test_servicex_dataset.py | Updates test mocks to include code generator configuration |
| tests/test_dataset.py | Updates test mocks to include code generator configuration |
| tests/test_databinder.py | Updates test structure and adds validation for error messages with code generator lists |
| tests/test_code_generator_fetch.py | Adds new tests covering cached vs uncached transform paths |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Feat/auto bump deploy key
Minor UI update to make the unites look a little better when printing out dataset sizes.
Update pyproject.toml back to 3.2.2 after GHA test
Add missing help text for various cli commands.
updates: - [github.com/pre-commit/pre-commit-hooks: v5.0.0 → v6.0.0](pre-commit/pre-commit-hooks@v5.0.0...v6.0.0) - [github.com/psf/black: 25.1.0 → 25.9.0](psf/black@25.1.0...25.9.0)
BenGalewsky
left a comment
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.
I don't think the code in query_core is correct?
Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.12.4 to 1.13.0. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.12.4...v1.13.0) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.13.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
…654) * add display_results option to deliver and --hide-results flag to cli
Bumps the actions group with 3 updates in the / directory: [actions/checkout](https://github.com/actions/checkout), [actions/setup-python](https://github.com/actions/setup-python) and [actions/upload-pages-artifact](https://github.com/actions/upload-pages-artifact). Updates `actions/checkout` from 4 to 5 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) Updates `actions/setup-python` from 5 to 6 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v5...v6) Updates `actions/upload-pages-artifact` from 3 to 4 - [Release notes](https://github.com/actions/upload-pages-artifact/releases) - [Commits](actions/upload-pages-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/setup-python dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/upload-pages-artifact dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]>
* change coverage options to match github build steps * remove erroneous pyproject attribute * remove branch attribute from pyproject
…ttps://github.com/ssl-hep/ServiceX_frontend into codex/refactor-servicex-client-and-query-handling
BenGalewsky
left a comment
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.
Cheers
* List supported code generators in NameError * re-implementing Push commit and commit version-bump * Update version to 0.0.1b * Adding deploy keys instead of Github tokens * Adding workflow and reseting version to master * Update version to 3.2.2b * version reset to 3.2.2 after release test * Handle None query input (#642) * Add dataset list name filtering * Merge pull request #652 from ssl-hep/feat/auto-bump-deploy-key Feat/auto bump deploy key * Update version to 3.2.2test * Update version to 0.0.1a * Update pyproject.toml back to 3.2.2 after GHA test * Use TB, MB, and GB for units as appropriate (#655) Minor UI update to make the unites look a little better when printing out dataset sizes. * Merge pull request #660 from ssl-hep/fix/set-version-3.2.2 Update pyproject.toml back to 3.2.2 after GHA test * Allow deleting multiple datasets (#656) * Add help text for codegen and datasets CLI (#639) Add missing help text for various cli commands. * feat: show human readable cache sizes * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/pre-commit/pre-commit-hooks: v5.0.0 → v6.0.0](pre-commit/pre-commit-hooks@v5.0.0...v6.0.0) - [github.com/psf/black: 25.1.0 → 25.9.0](psf/black@25.1.0...25.9.0) * Bump pypa/gh-action-pypi-publish in /.github/workflows Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.12.4 to 1.13.0. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.12.4...v1.13.0) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.13.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> * add display_results option to deliver and --hide-results flag to cli (#654) * add display_results option to deliver and --hide-results flag to cli * Bump the actions group across 1 directory with 3 updates Bumps the actions group with 3 updates in the / directory: [actions/checkout](https://github.com/actions/checkout), [actions/setup-python](https://github.com/actions/setup-python) and [actions/upload-pages-artifact](https://github.com/actions/upload-pages-artifact). Updates `actions/checkout` from 4 to 5 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) Updates `actions/setup-python` from 5 to 6 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v5...v6) Updates `actions/upload-pages-artifact` from 3 to 4 - [Release notes](https://github.com/actions/upload-pages-artifact/releases) - [Commits](actions/upload-pages-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/setup-python dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/upload-pages-artifact dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]> * change coverage options to match github build steps (#622) * change coverage options to match github build steps * remove erroneous pyproject attribute * remove branch attribute from pyproject * List supported code generators in NameError * Merge branch 'codex/refactor-servicex-client-and-query-handling' of https://github.com/ssl-hep/ServiceX_frontend into codex/refactor-servicex-client-and-query-handling
Goal: make sure no internet is needed for a cached query
Summary
Testing
black tests/test_databinder.py servicex/query_core.pyflake8 tests/test_databinder.py servicex/query_core.pypytesthttps://chatgpt.com/codex/tasks/task_e_689df04bede4832088ce0db7ccf3044b