Fixes #10013: Implement first stage of NoSQL profiler

[sushi30]

Describe your changes:

Fixes #10013

1. Implemented the NoSQLProfilerInterface as an entrypoint for the nosql profiler.
2. Added the NoSQLMetric as an abstract class.
3. Implemented the interface for the MongoDB database source.
4. Implemented an e2e test using testcontainers.

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.

[github-actions]

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]

I think this is a great idea. Did not know we had that for Python too. Would be worth a small discussion with the team to see where else we can use it 🚀

[sushi30]

absolutely. moving forward we can also use it to setup the OM instance in the tests instead of relying on whatever's running on our local machine.

[TeddyCr]

Can you set it up globally, (i.e. you only create it once at the beginning of the test session?

[sushi30]

Its possible with some hacking. Best practice is to abstract the setup to a function and start it with with every test suite (class) using setUpClass so that state is not corrupted between tests. The image can be cached locally so it should be low overhead.

[TeddyCr]

I see, It might add a bit of overhead if we need to start the server for every tests. 🤔

[harshach]

in backend, we start the containers for each Test Class

[github-actions]

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.

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	61.6% (31443/51047)	40.59% (12542/30898)	40.91% (3862/9441)

[sushi30]

anybody know why py_format_check is failing for files I have not changed?

ingestion/src/metadata/great_expectations/action.py:110:4: W0237: Parameter 'expectation_suite_identifier' has been renamed to 'payload' in overriding 'OpenMetadataValidationAction._run' method (arguments-renamed)
ingestion/src/metadata/great_expectations/action.py:110:4: W0237: Parameter 'checkpoint_identifier' has been renamed to 'expectation_suite_identifier' in overriding 'OpenMetadataValidationAction._run' method (arguments-renamed)

[github-actions]

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.

[TeddyCr]

OpenMetadataValidationAction

You should check the parent method signature (it looks like some parameters do not use the same name in the method of the children class. Weird though, as you have not touched that.

[TeddyCr]

We should not raise in the loop. The idea is that if we fail to compute 1 metric then we should log the error but continue to compute the rest of the metrics so we don't stop the pipeline mid way.

[sushi30]

For the PandasProfilerInterface, it appears that a single error also breaks the loop:

OpenMetadata/ingestion/src/metadata/profiler/interface/pandas/profiler_interface.py

Lines 144 to 153 in 9a4a9df

	try:
	row_dict = {}
	df_list = [df.where(pd.notnull(df), None) for df in runner]
	for metric in metrics:
	row_dict[metric.name()] = metric().df_fn(df_list)
	return row_dict
	except Exception as exc:
	logger.debug(traceback.format_exc())
	logger.warning(f"Error trying to compute profile for {exc}")
	raise RuntimeError(exc)

Should the errors be registered anywhere (other than log)? Should an exception be thrown at the end of the loop?

[TeddyCr]

@sushi30 thanks for the PR. Letf a few comments

[sushi30]

OpenMetadataValidationAction

You should check the parent method signature (it looks like some parameters do not use the same name in the method of the children class

@TeddyCr

The files which are being reported were not changed in this delivery (neither OpenMetadataValidationAction class). I dont mind fixing the linting issue but I it is a bit strange that it is getting reported here. I also dont see any breaking change in great-expectations

[sushi30]

re: #15189 (comment)

It appears gx have the same issue with their linter. I will add an ignore comment like they did...

https://github.com/great-expectations/great_expectations/blame/0.18.8/great_expectations/checkpoint/actions.py#L374C16-L374C83

[sushi30]

Do I need to add any migration after changing this schema?

[TeddyCr]

Yes, you will need to add that key to the connection. You can either handle it here or do it in a separate PR -- up to you.

[TeddyCr]

re: #15189 (comment)

It appears gx have the same issue with their linter. I will add an ignore comment like they did...

https://github.com/great-expectations/great_expectations/blame/0.18.8/great_expectations/checkpoint/actions.py#L374C16-L374C83

Awesome. Thank you

[github-actions]

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.

[github-actions]

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.

[TeddyCr]

why do we need to return a function and not just None?

[sushi30]

The method is returning a Callable:

https://github.com/open-metadata/OpenMetadata/pull/15189/files/74c2abd3de021f11d8b1630399605288260a007d..1733b9af92e92f317bd518855337da94f6699b76#diff-e439fc0df5e00e522353c3ee002acdf38e1320efcd148877a5ed01accee72279R162

If it returns a bare None it will need to handle a branch to to avoid TypeError: 'NoneType' object is not callable.

[TeddyCr]

Thanks @sushi30 LGTM. I just left a comment, you can run make py_format_check to check where the style is failing Once this passes we can merge it in 😊

[sushi30]

@TeddyCr thanks. Should I take any action on this discussion or are we good?

[sonarcloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[sonarcloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
2 New issues

Measures
0 Security Hotspots
84.9% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarCloud