-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677
Merged
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c0cad8c
Moved more recipes into ingestion/Makefile
TomBushell aab2823
Removed some recipes into ingestion/Makefile and added import statement
TomBushell dd78638
Modified file paths so that 'make generate' works from the ingestion …
TomBushell e99660c
Modified checks for current directory
TomBushell 96f2a23
Fixed function names to be in snake case
TomBushell 95e4197
Merge branch 'main' into issue-7858
pmbrull f16fb02
Reverted function names back to camel case
TomBushell c8ddddb
Reverted changes to js_antlr and py_antlr and moved generate command …
TomBushell b1c9a40
Updated run_ometa_integration_testsrecipe in ingestion/Makefile
TomBushell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
DIRECTORY_NAME := $(notdir $(CURDIR)) | ||
PY_SOURCE ?= ./src | ||
|
||
ifeq (ingestion,$(DIRECTORY_NAME)) | ||
INGESTION_DIR := . | ||
ROOT_DIR := .. | ||
else | ||
INGESTION_DIR := ingestion | ||
ROOT_DIR := . | ||
endif | ||
|
||
.PHONY: install | ||
install: ## Install the ingestion module to the current environment | ||
python -m pip install $(INGESTION_DIR)/ | ||
|
||
.PHONY: install_dev | ||
install_dev: ## Install the ingestion module with dev dependencies | ||
python -m pip install "$(INGESTION_DIR)[dev]/" | ||
|
||
.PHONY: install_test | ||
install_test: ## Install the ingestion module with test dependencies | ||
python -m pip install "$(INGESTION_DIR)[test]/" | ||
|
||
.PHONY: install_all | ||
install_all: ## Install the ingestion module with all dependencies | ||
python -m pip install "$(INGESTION_DIR)[all]/" | ||
|
||
.PHONY: install_apis | ||
install_apis: ## Install the REST APIs module to the current environment | ||
python -m pip install $(ROOT_DIR)openmetadata-airflow-apis/ | ||
|
||
.PHONY: lint | ||
lint: ## Run pylint on the Python sources to analyze the codebase | ||
PYTHONPATH="${PYTHONPATH}:$(INGESTION_DIR)/plugins" find $(PY_SOURCE) -path $(PY_SOURCE)/metadata/generated -prune -false -o -type f -name "*.py" | xargs pylint | ||
|
||
.PHONY: precommit_install | ||
precommit_install: ## Install the project's precommit hooks from .pre-commit-config.yaml | ||
@echo "Installing pre-commit hooks" | ||
@echo "Make sure to first run install_test first" | ||
pre-commit install | ||
|
||
.PHONY: py_format | ||
py_format: ## Run black and isort to format the Python codebase | ||
pycln $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --extend-exclude $(PY_SOURCE)/metadata/generated --all | ||
isort $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --skip $(PY_SOURCE)/metadata/generated --skip $(INGESTION_DIR)/env --skip $(INGESTION_DIR)/build --skip $(ROOT_DIR)/openmetadata-airflow-apis/build --profile black --multi-line 3 | ||
black $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --extend-exclude $(PY_SOURCE)/metadata/generated | ||
|
||
.PHONY: py_format_check | ||
py_format_check: ## Check if Python sources are correctly formatted | ||
pycln $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --diff --extend-exclude $(PY_SOURCE)/metadata/generated --all | ||
isort --check-only $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --skip $(PY_SOURCE)/metadata/generated --skip $(INGESTION_DIR)/build --skip $(ROOT_DIR)/openmetadata-airflow-apis/build --profile black --multi-line 3 | ||
black --check --diff $(INGESTION_DIR)/ $(ROOT_DIR)/openmetadata-airflow-apis/ --extend-exclude $(PY_SOURCE)/metadata/generated | ||
PYTHONPATH="${PYTHONPATH}:$(INGESTION_DIR)/plugins" pylint --fail-under=10 $(PY_SOURCE)/metadata --ignore-paths $(PY_SOURCE)/metadata/generated || (echo "PyLint error code $$?"; exit 1) | ||
|
||
.PHONY: unit_ingestion | ||
unit_ingestion: ## Run Python unit tests | ||
coverage run --rcfile $(INGESTION_DIR)/.coveragerc -a --branch -m pytest -c $(INGESTION_DIR)/setup.cfg --junitxml=$(INGESTION_DIR)/junit/test-results-unit.xml --ignore=$(INGESTION_DIR)/tests/unit/source $(INGESTION_DIR)/tests/unit | ||
|
||
## Ingestion tests & QA | ||
.PHONY: run_ometa_integration_tests | ||
run_ometa_integration_tests: ## Run Python integration tests | ||
coverage run --rcfile $(INGESTION_DIR)/.coveragerc -a --branch -m pytest -c $(INGESTION_DIR)/setup.cfg --junitxml=$(INGESTION_DIR)/junit/test-results-integration.xml $(INGESTION_DIR)/tests/integration/ometa ingestion/tests/integration/orm_profiler $(INGESTION_DIR)/tests/integration/test_suite $(INGESTION_DIR)/tests/integration/data_insight $(INGESTION_DIR)/tests/integration/lineage | ||
|
||
.PHONY: run_python_tests | ||
run_python_tests: ## Run all Python tests with coverage | ||
coverage erase | ||
$(MAKE) unit_ingestion | ||
$(MAKE) run_ometa_integration_tests | ||
coverage report --rcfile $(INGESTION_DIR)/.coveragerc || true | ||
|
||
.PHONY: sonar_ingestion | ||
sonar_ingestion: ## Run the Sonar analysis based on the tests results and push it to SonarCloud | ||
docker run \ | ||
--rm \ | ||
-e SONAR_HOST_URL="https://sonarcloud.io" \ | ||
-e SONAR_SCANNER_OPTS="-Xmx1g" \ | ||
-e SONAR_LOGIN=$(token) \ | ||
-v ${PWD}/$(INGESTION_DIR):/usr/src \ | ||
sonarsource/sonar-scanner-cli \ | ||
-Dproject.settings=sonar-project.properties | ||
|
||
.PHONY: run_apis_tests | ||
run_apis_tests: ## Run the openmetadata airflow apis tests | ||
coverage erase | ||
coverage run --rcfile $(ROOT_DIR)/openmetadata-airflow-apis/.coveragerc -a --branch -m pytest --junitxml=$(ROOT_DIR)/openmetadata-airflow-apis/junit/test-results.xml $(ROOT_DIR)/openmetadata-airflow-apis/tests | ||
coverage report --rcfile $(ROOT_DIR)/openmetadata-airflow-apis/.coveragerc | ||
|
||
|
||
.PHONY: coverage_apis | ||
coverage_apis: ## Run the python tests on openmetadata-airflow-apis | ||
$(MAKE) run_apis_tests | ||
coverage xml --rcfile $(ROOT_DIR)/openmetadata-airflow-apis/.coveragerc -o $(ROOT_DIR)/openmetadata-airflow-apis/coverage.xml | ||
sed -e "s/$(shell python -c "import site; import os; from pathlib import Path; print(os.path.relpath(site.getsitepackages()[0], str(Path.cwd())).replace('/','\/'))")\///g" $(INGESTION_DIR)/openmetadata-airflow-apis/coverage.xml >> $(INGESTION_DIR)/openmetadata-airflow-apis/ci-coverage.xml | ||
|
||
## Ingestion models generation | ||
.PHONY: generate | ||
generate: ## Generate the pydantic models from the JSON Schemas to the ingestion module | ||
@echo "Running Datamodel Code Generator" | ||
@echo "Make sure to first run the install_dev recipe" | ||
rm -rf $(INGESTION_DIR)/src/metadata/generated | ||
mkdir -p $(INGESTION_DIR)/src/metadata/generated | ||
python $(ROOT_DIR)/scripts/datamodel_generation.py | ||
$(MAKE) -C $(ROOT_DIR) py_antlr js_antlr | ||
$(MAKE) install | ||
|
||
.PHONY: coverage | ||
coverage: ## Run all Python tests and generate the coverage XML report | ||
$(MAKE) run_python_tests | ||
coverage xml --rcfile $(INGESTION_DIR)/.coveragerc -o $(INGESTION_DIR)/coverage.xml || true | ||
sed -e "s/$(shell python -c "import site; import os; from pathlib import Path; print(os.path.relpath(site.getsitepackages()[0], str(Path.cwd())).replace('/','\/'))")/src/g" $(INGESTION_DIR)/coverage.xml >> $(INGESTION_DIR)/ci-coverage.xml |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll need to double-check why the server is unable to start. I am not sure if we might be overlooking anything by updating this method names since those are related to ANTLR.
I'd suggest to revert this changes and see if the py-tests pass properly, since the major topic in the github issue was about cleaning up the Makefile (which is beautifully done here already).
Thanks