Use lazy import for generate_mapping_docs by moving it in setup function #2188
Use lazy import for generate_mapping_docs by moving it in setup function #2188pankajastro wants to merge 2 commits into
generate_mapping_docs by moving it in setup function #2188Conversation
generate_mapping_docs by moving it in function setupgenerate_mapping_docs by moving it in function setup
generate_mapping_docs by moving it in function setupgenerate_mapping_docs by moving it in setup function
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2188 +/- ##
==========================================
+ Coverage 97.46% 97.84% +0.38%
==========================================
Files 94 94
Lines 6038 6038
==========================================
+ Hits 5885 5908 +23
+ Misses 153 130 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves the documentation build process by converting the generate_mapping_docs import from an eager top-level import to a lazy import within a Sphinx setup function. This change allows the documentation to build gracefully even when Airflow dependencies are not installed, making local development easier.
Key Changes:
- Moved
generate_mapping_docsimport and execution from module-level to inside a Sphinxsetupfunction - Added error handling to catch and warn about missing dependencies instead of failing the build
- Made the paths explicit by constructing absolute paths to templates and output directories
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Make sure Airflow is installed (pip install -r docs/requirements.txt). " | ||
| "Documentation will be built without profile mapping pages.", | ||
| UserWarning, | ||
| ) |
There was a problem hiding this comment.
The Sphinx setup function should return a metadata dictionary. This is a best practice that helps Sphinx understand the extension's requirements and capabilities. Consider adding a return statement at the end:
return {
'version': '0.1',
'parallel_read_safe': True,
'parallel_write_safe': True,
}| ) | |
| ) | |
| return { | |
| 'version': '0.1', | |
| 'parallel_read_safe': True, | |
| 'parallel_write_safe': True, | |
| } |
| # If Airflow is not available, skip generating mapping docs | ||
| # This can happen during local development if dependencies aren't installed | ||
| import warnings | ||
|
|
||
| warnings.warn( | ||
| f"Could not generate mapping docs: {e}. " | ||
| "Make sure Airflow is installed (pip install -r docs/requirements.txt). " |
There was a problem hiding this comment.
The error message says "If Airflow is not available" but the actual import is from docs.generate_mappings. The import could fail for reasons other than Airflow not being available (e.g., missing jinja2). Consider making the error message more accurate:
# If dependencies are not available, skip generating mapping docs
# This can happen during local development if dependencies aren't fully installed| # If Airflow is not available, skip generating mapping docs | |
| # This can happen during local development if dependencies aren't installed | |
| import warnings | |
| warnings.warn( | |
| f"Could not generate mapping docs: {e}. " | |
| "Make sure Airflow is installed (pip install -r docs/requirements.txt). " | |
| # If required dependencies are not available, skip generating mapping docs | |
| # This can happen during local development if all documentation dependencies aren't installed | |
| import warnings | |
| warnings.warn( | |
| f"Could not generate mapping docs: {e}. " | |
| "Make sure all documentation dependencies are installed (pip install -r docs/requirements.txt). " |
|
The CI is green now, so we do not need this |
closes: https://github.com/astronomer/oss-integrations-private/issues/286