-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enhancements to CI Workflows and Python Module Initialization with Minor Fixes #1061
Open
kasinadhsarma
wants to merge
46
commits into
google:master
Choose a base branch
from
VishwamAI:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add fail-fast: false to prevent cancellations - Fix typos in artifact names and make them unique - Add proper error handling and verbosity to Python tests - Fix gather-digests job name and make it more resilient - Add proper ARM64 skip configuration
- Add pip, setuptools, and wheel upgrades - Install build and pytest dependencies explicitly - Set PYTHONPATH and LD_LIBRARY_PATH for proper library discovery - Ensure proper environment setup before building Python wrapper
- Add comprehensive permissions at job level to match workflow level - Include write permissions for pull-requests, actions, and checks - Ensure proper execution of pull_request_target events
…per environment setup
- Remove push triggers to focus on pull_request_target - Add workflow_dispatch for manual triggering - Improve concurrency settings with SHA-based grouping - Add explicit GITHUB_TOKEN environment variable - Simplify permissions inheritance - Add conditional to only run on forks and manual triggers
…ent circular imports
…t and simplify checkout configuration
- Remove Windows-specific configurations - Add environment variables at workflow level - Add verbose output for debugging - Add explicit dependency installation
- Add error handling in _load_sentencepiece() - Create separate registration functions - Move registrations to end of file - Add centralized initialization function
- Register immutable classes before processor classes - Add explicit error message for module load failures - Remove silent error handling in _load_sentencepiece
- Add lazy initialization for immutable classes - Defer SWIG registrations until after class definitions - Improve error handling for module loading and registration - Fix circular import issues in module initialization
Fix GitHub Actions workflow issues
- Add global initialization flag to prevent double registration - Remove premature class registration - Improve error handling with specific error messages - Ensure proper registration order for immutable classes
- Add proper state tracking for module loading - Prevent circular imports during initialization - Add dependency checks for SWIG registrations - Improve error handling for module loading - Ensure proper registration order for all classes
- Add state tracking with _initialized flag - Implement safe lazy loading with _ensure_initialized - Add SWIG module availability checks - Remove direct SWIG calls from __init__ - Improve error handling for module loading - Prevent circular imports during initialization
- Add registration state tracking to prevent circular imports - Modify _load_sentencepiece to handle registration phase - Add checks for required SWIG registration functions - Improve error handling in SentencePieceNormalizer - Restructure registration sequence for proper dependency order
- Add module load attempt tracking to prevent circular imports - Improve registration phase state management - Add proper cleanup of state flags on errors - Ensure proper initialization sequence for SWIG classes
- Use absolute imports to prevent path-related issues - Add proper path resolution for src directory - Ensure module initialization happens after path setup
- Add early verification of required SWIG registration functions - Improve error messages for missing registration functions - Prevent silent failures during module initialization
- Add registration lock to prevent circular imports - Move SWIG registrations to dedicated function - Improve error handling and state management - Remove redundant function verification
- Separate module loading from registration phase - Add two-phase initialization in _register_all_classes - Remove registration lock from _load_sentencepiece - Improve error handling for module loading
fix workflow issues/2745
- Add loading lock to prevent circular imports - Improve error handling with proper cleanup - Add retry mechanism for SWIG registrations - Ensure proper initialization order for all classes
fix: Implement robust module loading and registration sequence
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes significant updates to the CI workflows and some minor changes to the Python setup and documentation. The most important changes include improvements to the CI build process, updates to the Python module initialization, and minor fixes.
CI Workflow Improvements:
.github/workflows/cmake.yml
: Enhanced the build process by adding concurrency controls, environment variables, and simplifying the CMake configuration steps. The workflow now supports multiple platforms and includes steps for dependency installation and test execution..github/workflows/wheel.yml
: Added concurrency controls, updated permissions, and improved the build matrix. The workflow now skips specific tests and includes more detailed logging for the build process. [1] [2] [3] [4] [5] [6]Python Module Initialization:
python/setup.py
: Added the source directory to the Python path and imported the version directly from the package. [1] [2]python/src/sentencepiece/_init.py
: Created a new initialization file to handle the proper setup of import paths and environment variables for the SentencePiece module.Minor Fixes:
README.md
: Updated the link to the Sennrich et al. paper.python/src/sentencepiece/sentencepiece.i
: Moved the import of the version to ensure proper initialization. [1] [2]python/test/sentencepiece_test.py
: Adjusted the test setup to ensure proper module initialization using absolute imports.