Skip to content

Commit

Permalink
ci: add job to check imports (#8594)
Browse files Browse the repository at this point in the history
* try checking imports

* clarify error message

* better fmt

* do not show complete list of successfully imported packages

* refinements

* relnote

* add missing forward references

* better function name

* linting

* fix linting

* Update .github/utils/check_imports.py

Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>

---------

Co-authored-by: Madeesh Kannan <shadeMe@users.noreply.github.com>
  • Loading branch information
anakin87 and shadeMe authored Nov 29, 2024
1 parent 163c06f commit de7099e
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 73 deletions.
74 changes: 74 additions & 0 deletions .github/utils/check_imports.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import os
import sys
import importlib
import traceback
from haystack import logging # pylint: disable=unused-import # this is needed to avoid circular imports

def validate_package_imports(directory: str):
"""
Recursively search for directories with __init__.py and import them.
"""
imported = []
failed = []

sys.path.insert(0, directory)

for root, _, files in os.walk(directory):
# skip directories without __init__.py
if not '__init__.py' in files:
continue

init_path = os.path.join(root, '__init__.py')

# skip haystack/__init__.py to avoid circular imports
if init_path.endswith(f'{directory}/__init__.py'):
continue

# convert filesystem path to Python module name
relative_path = os.path.relpath(root, directory)
module_name = relative_path.replace(os.path.sep, '.')
if module_name == '.':
module_name = os.path.basename(directory)

try:
importlib.import_module(module_name)
imported.append(module_name)
except:
failed.append({
'module': module_name,
'traceback': traceback.format_exc()
})

return imported, failed


def main():
"""
This script checks that all Haystack packages can be imported successfully.
This can detect several issues.
One example is forgetting to use a forward reference for a type hint coming
from a lazy import.
"""
print("Checking imports from Haystack packages...")
imported, failed = validate_package_imports(directory="haystack")

if not imported:
print("\nNO PACKAGES WERE IMPORTED")
sys.exit(1)

print(f"\nSUCCESSFULLY IMPORTED {len(imported)} PACKAGES")

if failed:
print(f"\nFAILED TO IMPORT {len(failed)} PACKAGES:")
for fail in failed:
print(f" - {fail['module']}")

print("\nERRORS:")
for fail in failed:
print(f" - {fail['module']}\n")
print(f" {fail['traceback']}\n\n")
sys.exit(1)


if __name__ == '__main__':
main()
47 changes: 47 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,53 @@ jobs:
- "branch:${{ github.ref_name }}"
- "url:https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
check-imports:
needs: format
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- uses: actions/setup-python@v5
with:
python-version: "${{ env.PYTHON_VERSION }}"

- name: Install Hatch
run: pip install hatch==${{ env.HATCH_VERSION }}

- name: Check imports
run: hatch run python .github/utils/check_imports.py

- name: Calculate alert data
id: calculator
shell: bash
if: (success() || failure()) && github.ref_name == 'main'
run: |
if [ "${{ job.status }}" = "success" ]; then
echo "alert_type=success" >> "$GITHUB_OUTPUT";
else
echo "alert_type=error" >> "$GITHUB_OUTPUT";
fi
- name: Send event to Datadog
if: (success() || failure()) && github.ref_name == 'main'
uses: masci/datadog@v1
with:
api-key: ${{ secrets.CORE_DATADOG_API_KEY }}
api-url: https://api.datadoghq.eu
events: |
- title: "${{ github.workflow }} workflow"
text: "Job ${{ github.job }} in branch ${{ github.ref_name }}"
alert_type: "${{ steps.calculator.outputs.alert_type }}"
source_type_name: "Github"
host: ${{ github.repository_owner }}
tags:
- "project:${{ github.repository }}"
- "job:${{ github.job }}"
- "run_id:${{ github.run_id }}"
- "workflow:${{ github.workflow }}"
- "branch:${{ github.ref_name }}"
- "url:https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
unit-tests:
name: Unit / ${{ matrix.os }}
needs: format
Expand Down
4 changes: 2 additions & 2 deletions haystack/components/connectors/openapi_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def _parse_message(self, message: ChatMessage) -> List[Dict[str, Any]]:
)
return function_payloads

def _authenticate_service(self, openapi_service: OpenAPI, credentials: Optional[Union[dict, str]] = None):
def _authenticate_service(self, openapi_service: "OpenAPI", credentials: Optional[Union[dict, str]] = None):
"""
Authentication with an OpenAPI service.
Expand Down Expand Up @@ -236,7 +236,7 @@ def _authenticate_service(self, openapi_service: OpenAPI, credentials: Optional[
f"for it. Check the service configuration and credentials."
)

def _invoke_method(self, openapi_service: OpenAPI, method_invocation_descriptor: Dict[str, Any]) -> Any:
def _invoke_method(self, openapi_service: "OpenAPI", method_invocation_descriptor: Dict[str, Any]) -> Any:
"""
Invokes the specified method on the OpenAPI service.
Expand Down
4 changes: 0 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,6 @@ extra-dependencies = [
# Structured logging
"structlog",

# Looking for missing imports
"isort",
"pyproject-parser",

# Test
"pytest",
"pytest-bdd",
Expand Down
6 changes: 6 additions & 0 deletions releasenotes/notes/check-imports-8a2aa18c73e5c492.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
enhancements:
- |
Add a testing job to check that all packages can be imported successfully.
This should help detecting several issues, such as forgetting to use a
forward reference for a type hint coming from a lazy import.
67 changes: 0 additions & 67 deletions test/test_imports.py

This file was deleted.

0 comments on commit de7099e

Please sign in to comment.