Skip to content
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
wants to merge 46 commits into
base: master
Choose a base branch
from

Conversation

kasinadhsarma
Copy link

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:

- 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
- 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
- Remove Windows-specific configurations
- Add environment variables at workflow level
- Add verbose output for debugging
- Add explicit dependency installation
devin-ai-integration bot and others added 16 commits October 24, 2024 14:16
- 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
- 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
- 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
Copy link

google-cla bot commented Oct 24, 2024

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant