refactor(ingestion): looker source migration to use SDKv2 entities #14693
Conversation
github-actions
bot
added
the
ingestion
github-actions
bot
added
the
community-contribution
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
datahub-cyborg
bot
added
the
needs-review
|
✅ Meticulous spotted 0 visual differences across 1472 screens tested: view results. Meticulous evaluated ~8 hours of user flows against your PR. Expected differences? Click here. Last updated for commit de36060. This comment will update as new commits are pushed. |
Bundle ReportBundle size has no change ✅ |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
actionlint
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:5:134: Double quote to prevent globbing and word splitting [shellcheck]
datahub/.github/workflows/docker-unified.yml
Line 273 in e10d20a
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:117: Double quote to prevent globbing and word splitting [shellcheck]
datahub/.github/workflows/docker-unified.yml
Line 321 in e10d20a
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:2:73: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:50: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:6:27: Double quote to prevent globbing and word splitting [shellcheck]
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2001:style:2:7: See if you can use ${variable//search/replace} instead [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:20: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting [shellcheck]
| - name: Download build Metadata for latest head build | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | |
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:51: Double quote to prevent globbing and word splitting [shellcheck]
| - name: Collect image:tag from build log | ||
| id: collect-images | ||
| run: | |
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2116:style:4:22: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo' [shellcheck]
| - name: Collect image:tag from build log | ||
| id: collect-images | ||
| run: | |
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:4:29: Double quote to prevent globbing and word splitting [shellcheck]
| - name: Download build Metadata for latest head build | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | |
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:51: Double quote to prevent globbing and word splitting [shellcheck]
| - name: Collect image:tag from build log | ||
| id: collect-images | ||
| run: | |
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2116:style:4:22: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo' [shellcheck]
| - name: Download build Metadata for latest head build | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | |
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:3:51: Double quote to prevent globbing and word splitting [shellcheck]
| - name: Collect image:tag from build log | ||
| id: collect-images | ||
| run: | |
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2116:style:4:22: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo' [shellcheck]
| - name: Collect image:tag from build log | ||
| id: collect-images | ||
| run: | |
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:4:29: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/docker-unified.yml
Outdated
| @@ -608,7 +610,7 @@ jobs: | |||
| path: ${{ github.workspace }}/build | |||
|
|
|||
| - name: Push images from depot builder | |||
| if: ${{ steps.tests_passed.outputs.tests_passed == 'true' && needs.setup.outputs.use_depot_cache == 'true' && (needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' ) }} | |||
| if: ${{ steps.tests_passed.outputs.tests_passed == 'true' && needs.setup.outputs.use_depot_cache == 'true' && needs.setup.outputs.publish == 'true' }} | |||
| run: | | |||
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:5:102: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/docker-unified.yml
Outdated
| @@ -608,7 +610,7 @@ jobs: | |||
| path: ${{ github.workspace }}/build | |||
|
|
|||
| - name: Push images from depot builder | |||
| if: ${{ steps.tests_passed.outputs.tests_passed == 'true' && needs.setup.outputs.use_depot_cache == 'true' && (needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' ) }} | |||
| if: ${{ steps.tests_passed.outputs.tests_passed == 'true' && needs.setup.outputs.use_depot_cache == 'true' && needs.setup.outputs.publish == 'true' }} | |||
| run: | | |||
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:5:116: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/airflow-plugin.yml
Outdated
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
property "extra_pip_extras" is not defined in object type {extra_pip_constraints: string; extra_pip_requirements: string; python-version: number} [expression]
.github/workflows/build-and-test.yml
Outdated
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2006:style:1:21: Use $(...) notation instead of legacy backticks ... [shellcheck]
datahub/.github/workflows/build-and-test.yml
Line 126 in e10d20a
.github/workflows/build-and-test.yml
Outdated
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:360: Double quote to prevent globbing and word splitting [shellcheck]
datahub/.github/workflows/build-and-test.yml
Line 126 in e10d20a
.github/workflows/build-and-test.yml
Outdated
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2006:style:2:22: Use $(...) notation instead of legacy backticks ... [shellcheck]
datahub/.github/workflows/build-and-test.yml
Line 126 in e10d20a
.github/workflows/build-and-test.yml
Outdated
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:2:155: Double quote to prevent globbing and word splitting [shellcheck]
datahub/.github/workflows/build-and-test.yml
Line 126 in e10d20a
.github/workflows/build-and-test.yml
Outdated
There was a problem hiding this comment.
📝 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:1:63: Double quote to prevent globbing and word splitting [shellcheck]
datahub/.github/workflows/build-and-test.yml
Line 130 in e10d20a
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2129:style:2:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects [shellcheck]
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2129:style:6:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects [shellcheck]
.github/workflows/docker-unified.yml
Outdated
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2129:style:2:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects [shellcheck]
datahub/.github/workflows/docker-unified.yml
Line 138 in e10d20a
.github/workflows/docker-unified.yml
Outdated
There was a problem hiding this comment.
🚫 [actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2129:style:7:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects [shellcheck]
datahub/.github/workflows/docker-unified.yml
Line 138 in e10d20a
askumar27
force-pushed
the
feature/ing-1025/looker-sdk-v2
branch
from
e10d20a to
3bb75ac
Compare
askumar27
force-pushed
the
feature/ing-1025/looker-sdk-v2
branch
from
de36060 to
5025983
Compare
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
| message="Dropped due to being a personal folder", | ||
| context=f"Dashboard ID: {dashboard_object.id}", | ||
| ) | ||
| assert dashboard_object.id is not None |
There was a problem hiding this comment.
Same thing here. We may have done this in the past but in general want to make our ingestion connectors more robust
metadata-ingestion/src/datahub/ingestion/source/looker/looker_source.py
Outdated
Show resolved
Hide resolved
…stance aspects and removed unused browse paths
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| self, folder: LookerFolder, include_current_folder: bool = True | ||
| ) -> Iterable[str]: | ||
| for ancestor in self.looker_api.folder_ancestors(folder_id=folder.id): | ||
| assert ancestor.id # to make the linter happy as `Folder` has id field marked optional - which is always returned by the API |
There was a problem hiding this comment.
Dangerous use of assert - low severity
When running Python in production in optimized mode, assert calls are not executed. This mode is enabled by setting the PYTHONOPTIMIZE command line flag. Optimized mode is usually ON in production. Any safety check done using assert will not be executed.
Remediation: Raise an exception instead of using assert.
View details in Aikido Security
…d related type hints for consistency.
| message="Dropped due to being a personal folder", | ||
| context=f"Dashboard ID: {dashboard_object.id}", | ||
| ) | ||
| assert dashboard_object.id is not None |
There was a problem hiding this comment.
Dangerous use of assert - low severity
When running Python in production in optimized mode, assert calls are not executed. This mode is enabled by setting the PYTHONOPTIMIZE command line flag. Optimized mode is usually ON in production. Any safety check done using assert will not be executed.
Remediation: Raise an exception instead of using assert.
View details in Aikido Security
refactor(ingestion): looker source migration to use SDKv2 entities
Changes
Test Files Updated
Key Test Updates
Breaking Changes