Fixes #16263 : Mode Ingestion not ingesting any records bug fix #16264

dixon2678 · 2024-05-15T05:38:29Z

Describe your changes:

I worked on changing the collections API get collection method by adding a filter=custom param because no records will be returned if this param is not included.

Reference Mode API docs

Type of change:

Bug fix
Breaking change (fix or feature that would cause existing functionality to not work as expected) -> I am unable to confirm whether the current users that uses existing deprecated Personal Token authentication method will be impacted.

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.

github-actions · 2024-05-15T05:38:52Z

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!

dixon2678 · 2024-05-15T05:40:34Z

ingestion/src/metadata/ingestion/source/dashboard/mode/client.py

@@ -75,7 +75,7 @@ def fetch_all_reports(self, workspace_name: str) -> Optional[list]:
            dict
        """
        all_reports = []
-        response_collections = self.client.get(f"/{workspace_name}/{COLLECTIONS}")
+        response_collections = self.client.get(f"/{workspace_name}/{COLLECTIONS}?filter=all")


filter=all param is added to get records from collections API

@dixon2678 this looks good to me does this need any special permissions?

@ulixius9 I have added my reply on the latest change 🙇‍♂️

github-actions · 2024-05-15T05:55:17Z

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 · 2024-05-15T06:21:45Z

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.

dixon2678 · 2024-05-15T06:53:43Z

ingestion/src/metadata/ingestion/source/dashboard/mode/client.py

@@ -75,7 +75,7 @@ def fetch_all_reports(self, workspace_name: str) -> Optional[list]:
            dict
        """
        all_reports = []
-        response_collections = self.client.get(f"/{workspace_name}/{COLLECTIONS}?filter=all")
+        response_collections = self.client.get(f"/{workspace_name}/{COLLECTIONS}?filter=custom")


Hi @ulixius9 I have made this change from all to custom

To answer your inquiry, all API Workspace tokens in Mode will have admin privileges by default (also only admins are able to create them) and according to the docs, it looks like granular permission control are not yet supported.

Personal tokens that support granular permssion control to Mode resources appears to be deprecated according to this docs, and no new personal tokens are able to be created as of now. This will be fully deprecated by August.

Several points when using the new Workspace Token:

At least in my case here, not specifying the filter parameter returns no collections. I have tried it with both Restricted and Unrestricted (default access) collections.

Specifying custom as a filter returns all collections in the Workspace, except Personal collections -> I think this is the most appropriate approach.

Specifying 'all' returns all collections including Personal collections.

However, there is no way that I could confirm whether the existing users that are still using the deprecated Personal tokens (with limited access, as opposed to the new Workspace token method that grants default admin access) are able to use the filter=custom params without any errors, as I am also unable to create a new Personal token in Mode.

Perhaps should we hold off this PR until the usage of exisitng Personal Tokens are fully deprecated in August 2024? @ulixius9

Thanks @dixon2678 for this details explanation, IMO we can approach it this way -> we introduce a new field in the service connection form so that user can configure this filter i.e wether they want an all, custom or no filter at all

this way it is possible for users to ingest or skip personal collections and also possible for them to use the deprecated version if they still decide to use is, what do you think about this approach?

and we can keep no filter as a default selection so it does not affect the existing users

dixon2678 · 2024-05-17T05:18:00Z

ingestion/src/metadata/ingestion/source/dashboard/mode/client.py

@@ -67,15 +67,22 @@ def __init__(self, config):
        )
        self.client = REST(client_config)

-    def fetch_all_reports(self, workspace_name: str) -> Optional[list]:
+    def fetch_all_reports(self, workspace_name: str, filter: Optional[str] = None) -> Optional[list]:


I have added filter as an argument with the default value of None (i.e. no filter). What do you think? @ulixius9

github-actions · 2024-05-17T05:23:31Z

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 · 2024-05-17T06:09:28Z

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.

sonarcloud · 2024-05-17T06:39:34Z

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Update collection method to include 'filter=all' params

6bf7049

dixon2678 requested a review from a team as a code owner May 15, 2024 05:38

github-actions bot added the Ingestion label May 15, 2024

dixon2678 commented May 15, 2024

View reviewed changes

dixon2678 mentioned this pull request May 15, 2024

[WITH PR] Mode Ingestion not ingesting any records #16263

Open

ulixius9 added the safe to test Add this label to run secure Github workflows on PRs label May 15, 2024

change filter=all to filter=custom

8556ddd

dixon2678 commented May 15, 2024

View reviewed changes

Mode client - add parameter filter

7edfdf0

dixon2678 commented May 17, 2024

View reviewed changes

Merge branch 'main' into update-mode-api-ingestion

b509f9c

dixon2678 marked this pull request as draft May 20, 2024 08:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #16263 : Mode Ingestion not ingesting any records bug fix #16264

Fixes #16263 : Mode Ingestion not ingesting any records bug fix #16264

dixon2678 commented May 15, 2024 •

edited

github-actions bot commented May 15, 2024

dixon2678 May 15, 2024

ulixius9 May 15, 2024

dixon2678 May 15, 2024

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

dixon2678 May 15, 2024 •

edited

ulixius9 May 16, 2024 •

edited

dixon2678 May 17, 2024

github-actions bot commented May 17, 2024

github-actions bot commented May 17, 2024

sonarcloud bot commented May 17, 2024

Fixes #16263 : Mode Ingestion not ingesting any records bug fix #16264

Are you sure you want to change the base?

Fixes #16263 : Mode Ingestion not ingesting any records bug fix #16264

Conversation

dixon2678 commented May 15, 2024 • edited

Describe your changes:

Type of change:

Checklist:

github-actions bot commented May 15, 2024

dixon2678 May 15, 2024

Choose a reason for hiding this comment

ulixius9 May 15, 2024

Choose a reason for hiding this comment

dixon2678 May 15, 2024

Choose a reason for hiding this comment

github-actions bot commented May 15, 2024

github-actions bot commented May 15, 2024

dixon2678 May 15, 2024 • edited

Choose a reason for hiding this comment

ulixius9 May 16, 2024 • edited

Choose a reason for hiding this comment

dixon2678 May 17, 2024

Choose a reason for hiding this comment

github-actions bot commented May 17, 2024

github-actions bot commented May 17, 2024

sonarcloud bot commented May 17, 2024

Quality Gate passed for 'open-metadata-ingestion'

dixon2678 commented May 15, 2024 •

edited

dixon2678 May 15, 2024 •

edited

ulixius9 May 16, 2024 •

edited