Enforce Conda pyproject.toml metadata in CI#45012
Enforce Conda pyproject.toml metadata in CI#45012JennyPng wants to merge 3 commits intoAzure:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enforces that Python packages with stable releases on PyPI must include a [tool.azure-sdk-conda] section in their pyproject.toml file. This automation supports the Conda release process by ensuring service teams specify whether packages should be released individually or bundled.
Changes:
- Adds validation to check for Conda metadata in packages with stable PyPI versions
- Fixes logging calls to use the correct logger instance
- Removes unused import statement
|
|
||
| def verify_conda_section(package_dir: str, package_name: str) -> bool: | ||
| """Verify that packages with stable versions on PyPI have [tool.azure-sdk-conda] section in pyproject.toml.""" | ||
| if not has_stable_version_on_pypi(package_name): |
There was a problem hiding this comment.
The verify_conda_section function makes a PyPI API call for every package being verified (via has_stable_version_on_pypi). In a CI environment processing multiple packages, this could be inefficient. Consider caching the results of has_stable_version_on_pypi across multiple verify_conda_section calls, or batch the PyPI lookups if possible.
| def has_stable_version_on_pypi(package_name: str) -> bool: | ||
| """Check if the package has any stable (non-prerelease) version on PyPI.""" | ||
| try: | ||
| all_versions = retrieve_versions_from_pypi(package_name) | ||
| stable_versions = [Version(v) for v in all_versions if not Version(v).is_prerelease] | ||
| return len(stable_versions) > 0 | ||
| except Exception: | ||
| return False | ||
|
|
||
|
|
||
| def verify_conda_section(package_dir: str, package_name: str) -> bool: | ||
| """Verify that packages with stable versions on PyPI have [tool.azure-sdk-conda] section in pyproject.toml.""" | ||
| if not has_stable_version_on_pypi(package_name): | ||
| logger.info(f"Package {package_name} has no stable version on PyPI, skipping conda section check") | ||
| return True | ||
|
|
||
| pyproject_path = os.path.join(package_dir, "pyproject.toml") | ||
| if not os.path.exists(pyproject_path): | ||
| logger.error(f"Package {package_name} has a stable version on PyPI but is missing pyproject.toml") | ||
| return False | ||
|
|
||
| try: | ||
| with open(pyproject_path, "r", encoding="utf-8") as f: | ||
| content = f.read() | ||
|
|
||
| if "[tool.azure-sdk-conda]" not in content: | ||
| logger.error( | ||
| f"Package {package_name} has a stable version on PyPI but is missing " | ||
| "[tool.azure-sdk-conda] section in pyproject.toml" | ||
| ) | ||
| return False | ||
| return True | ||
| except Exception as e: | ||
| logger.error(f"Failed to read pyproject.toml for {package_name}: {e}") | ||
| return False |
There was a problem hiding this comment.
The new verify_conda_section and has_stable_version_on_pypi functions lack test coverage. The repository has comprehensive automated testing for verify_whl (see tests/test_metadata_verification.py), so these new functions should have corresponding tests. Consider adding test cases for: 1) packages with stable versions that have the conda section, 2) packages with stable versions missing the conda section, 3) packages without stable versions (should skip check), 4) packages missing pyproject.toml, and 5) error handling in has_stable_version_on_pypi.
| # Verify conda section for packages with stable versions on PyPI | ||
| if not verify_conda_section(package_dir, package_name): | ||
| logger.error( | ||
| "As part of releasing stable packages to Conda, the pyproject.toml must include a [tool.azure-sdk-conda] section and specify if the package should be released individually or bundled." |
There was a problem hiding this comment.
The error message duplicates information already provided by the logger.error call on lines 127-130. The message on line 297 should be removed or modified to add new information, such as a link to documentation on how to configure the conda section or an example configuration.
| "As part of releasing stable packages to Conda, the pyproject.toml must include a [tool.azure-sdk-conda] section and specify if the package should be released individually or bundled." | |
| "As part of releasing stable packages to Conda, the pyproject.toml must include a [tool.azure-sdk-conda] " | |
| "section and specify if the package should be released individually or bundled. For configuration guidance, " | |
| "see https://aka.ms/azsdk/python/conda-config, for example:\n" | |
| "[tool.azure-sdk-conda]\n" | |
| 'publish = "individual" # or "bundle"\n' | |
| 'bundle_group = "azure-ai" # optional, when using bundle' |
| if "[tool.azure-sdk-conda]" not in content: | ||
| logger.error( | ||
| f"Package {package_name} has a stable version on PyPI but is missing " | ||
| "[tool.azure-sdk-conda] section in pyproject.toml" | ||
| ) | ||
| return False | ||
| return True |
There was a problem hiding this comment.
The verification only checks for the presence of the [tool.azure-sdk-conda] section but doesn't validate that it contains the required fields. Based on existing pyproject.toml files (e.g., sdk/core/azure-core/pyproject.toml), packages should specify "in_bundle" and "bundle_name" fields. Consider validating that these required fields are present and properly configured to provide more helpful feedback to service teams.
| except Exception as e: | ||
| logger.error(f"Failed to read pyproject.toml for {package_name}: {e}") | ||
| return False |
There was a problem hiding this comment.
The bare Exception catch in the try-except block is too broad. It catches all exceptions including KeyboardInterrupt and SystemExit, which could mask serious issues. Consider catching specific exceptions like OSError or IOError for file operations, or at minimum use "except Exception as e" and re-raise after logging if it's not a recoverable error. The current implementation silently returns False for any exception, which could hide bugs.
| all_versions = retrieve_versions_from_pypi(package_name) | ||
| stable_versions = [Version(v) for v in all_versions if not Version(v).is_prerelease] | ||
| return len(stable_versions) > 0 | ||
| except Exception: |
There was a problem hiding this comment.
Similar to verify_conda_section, the bare Exception catch here is too broad and could hide bugs. The function returns False for any exception (including network errors, JSON parsing errors, etc.), which makes debugging difficult. Consider either catching specific exceptions or logging the exception before returning False so that failures are visible in CI logs.
| except Exception: | |
| except Exception as exc: | |
| logger.error("Failed to retrieve stable versions for %s from PyPI: %s", package_name, exc) |
| with open(pyproject_path, "r", encoding="utf-8") as f: | ||
| content = f.read() | ||
|
|
||
| if "[tool.azure-sdk-conda]" not in content: |
There was a problem hiding this comment.
The string search for "[tool.azure-sdk-conda]" may produce false positives if this string appears in comments or string literals within the pyproject.toml file. Consider parsing the TOML file properly to check for the actual section. The codebase already uses tomllib/tomli for TOML parsing (see ci_tools/parsing/parse_functions.py), so you could load the file and check if "azure-sdk-conda" exists in toml_dict.get("tool", {}).
1 participant
closes #44883
packages with stable releases are required to have a Conda tool section in their pyproject.toml to specify if packages are to be released individually or as part of a bundle
service teams should be responsible for specifying this information
existing stable packages have already been batch updated