Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677

TomBushell · 2023-10-22T07:09:35Z

Describe your changes:

Targets #7858.

I moved ingestion specific make commands in the root project Makefile into a new Makefile in the ingestion/ directory. These recipes can be run in either directory now. To do this I also modified some file paths in scripts/datamodel_code_generator.py.

Type of change:

Improvement

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

I have added tests around the new logic.
For connector/ingestion changes: I updated the documentation.

…directory

github-actions · 2023-10-22T07:10:12Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

ayush-shah · 2023-10-23T05:45:16Z

scripts/datamodel_generation.py

+ingestion_path = "./" if current_directory.endswith("OpenMetadata/ingestion") else "ingestion/"
+directory_root = "../" if current_directory.endswith("OpenMetadata/ingestion") else "./"


@TomBushell chances are the directory would be named as something else for different users, we could keep endswith("/ingestion") to determine the ingestion directory, what do you think about that?

Yep, this will work.

github-actions · 2023-10-23T05:52:38Z

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

ayush-shah

@TomBushell also, the PR has formatting issues, if you can run the commands given in the comment above and fix it, Thanks

github-actions · 2023-10-23T10:49:09Z

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

pmbrull · 2023-10-25T08:09:40Z

ingestion/src/metadata/antlr/split_listener.py

@@ -21,10 +21,10 @@ class FqnSplitListener(FqnListener):
    def __init__(self):
        self._list = []

-    def enterQuotedName(self, ctx: FqnParser.QuotedNameContext):
+    def enter_quoted_name(self, ctx: FqnParser.QuotedNameContext):


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

github-actions · 2023-10-25T13:39:36Z

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

pmbrull · 2023-10-27T07:32:05Z

hi @TomBushell have you been able to identify why the sample data ingestion is not working? Looks like the "Sample Data DAG is not reachable".

Let us know if you have any questions on how to identify this.

Running sh docker/run_local_docker.sh would redo the same process that the CI is trying here.

Thanks

TomBushell · 2023-10-27T10:17:55Z

hi @TomBushell have you been able to identify why the sample data ingestion is not working? Looks like the "Sample Data DAG is not reachable".

Let us know if you have any questions on how to identify this.

Running sh docker/run_local_docker.sh would redo the same process that the CI is trying here.

Thanks

@pmbrull I'm not sure yet why this is happening. I have run sh docker/run_local_docker.sh and there are no problems.

…back into root directory Makefile

sonarcloud · 2023-10-28T05:46:24Z

[open-metadata-ingestion] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

pmbrull · 2023-10-30T07:23:43Z

Ci looks good after merging main. Bringing this in, thanks~

TomBushell added 3 commits October 22, 2023 17:28

Moved more recipes into ingestion/Makefile

c0cad8c

Removed some recipes into ingestion/Makefile and added import statement

aab2823

Modified file paths so that 'make generate' works from the ingestion …

dd78638

…directory

TomBushell requested a review from a team as a code owner October 22, 2023 07:09

github-actions bot added the Ingestion label Oct 22, 2023

ayush-shah reviewed Oct 23, 2023

View reviewed changes

ayush-shah added the safe to test Add this label to run secure Github workflows on PRs label Oct 23, 2023

ayush-shah requested changes Oct 23, 2023

View reviewed changes

Modified checks for current directory

e99660c

TomBushell and others added 2 commits October 24, 2023 00:42

Fixed function names to be in snake case

96f2a23

Merge branch 'main' into issue-7858

95e4197

pmbrull reviewed Oct 25, 2023

View reviewed changes

Reverted function names back to camel case

f16fb02

TomBushell had a problem deploying to test October 25, 2023 13:31 — with GitHub Actions Failure

TomBushell added 2 commits October 28, 2023 15:26

Reverted changes to js_antlr and py_antlr and moved generate command …

c8ddddb

…back into root directory Makefile

Updated run_ometa_integration_testsrecipe in ingestion/Makefile

b1c9a40

pmbrull approved these changes Oct 30, 2023

View reviewed changes

pmbrull merged commit 33fe3b5 into open-metadata:main Oct 30, 2023
14 of 17 checks passed

pmbrull mentioned this pull request Nov 20, 2023

Fix #12436 - Migrate to pyproject.toml #14025

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677

Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677

TomBushell commented Oct 22, 2023

github-actions bot commented Oct 22, 2023

ayush-shah Oct 23, 2023

TomBushell Oct 23, 2023

github-actions bot commented Oct 23, 2023

ayush-shah left a comment

github-actions bot commented Oct 23, 2023

pmbrull Oct 25, 2023

github-actions bot commented Oct 25, 2023

pmbrull commented Oct 27, 2023

TomBushell commented Oct 27, 2023 •

edited

sonarcloud bot commented Oct 28, 2023

pmbrull commented Oct 30, 2023

		ingestion_path = "./" if current_directory.endswith("OpenMetadata/ingestion") else "ingestion/"
		directory_root = "../" if current_directory.endswith("OpenMetadata/ingestion") else "./"

Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677

Fixes #7858: Moved ingestion related make commands into Makefile in ingestion directory #13677

Conversation

TomBushell commented Oct 22, 2023

Describe your changes:

Type of change:

Checklist:

github-actions bot commented Oct 22, 2023

ayush-shah Oct 23, 2023

Choose a reason for hiding this comment

TomBushell Oct 23, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 23, 2023

ayush-shah left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 23, 2023

pmbrull Oct 25, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 25, 2023

pmbrull commented Oct 27, 2023

TomBushell commented Oct 27, 2023 • edited

sonarcloud bot commented Oct 28, 2023

pmbrull commented Oct 30, 2023

TomBushell commented Oct 27, 2023 •

edited