Skip to content
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

feat: create delete assets API #77

Merged
merged 46 commits into from
Aug 22, 2024
Merged

Conversation

luthfifahlevi
Copy link

@luthfifahlevi luthfifahlevi commented Aug 8, 2024

Background

Currently, there are two services for data discovery: Meteor and Compass. Meteor extracts assets from various sources (e.g., BigQuery tables, Optimus, Firehose, Dagger jobs) and syncs the metadata to Compass as the catalog service. However, currently, Compass has a lot of assets that no longer present and not synced. We need a housekeeping mechanism in Compass to automatically remove stale assets from the database.

Goals in this Repo:
Create delete assets API in Compass

Idea:
DeleteAssets as DELETE /v1/assets/: an async API that will return how many rows that will affect based on filter criteria, and delete those rows from PostgreSQL (assets and lineage) and Elasticsearch in asynchronous.

Request:

  • query_expr represent query expr for filter criteria: https://github.com/expr-lang/expr.
  • dry_run represent whether it only return how much affected rows (if set to true), or also perform deletion process in the background (default: false).

Response:
affected_rows represent how many rows that will affect based on filter criteria.

Current Approach:

  • Query Expr --> AST --> SQL query and ES query.
  • Use those query as WHERE clause in SQL and filter in ES query in deletion process, make this process in asynchronous.
  • Return the affected rows

@luthfifahlevi luthfifahlevi self-assigned this Aug 8, 2024
@luthfifahlevi luthfifahlevi marked this pull request as draft August 8, 2024 08:21
@luthfifahlevi luthfifahlevi changed the title feat: create delete assets api feat: create delete assets API Aug 8, 2024
@coveralls
Copy link

coveralls commented Aug 8, 2024

Pull Request Test Coverage Report for Build 10503449624

Details

  • 562 of 762 (73.75%) changed or added relevant lines in 14 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-1.1%) to 84.069%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/server/v1beta1/asset.go 20 22 90.91%
internal/store/elasticsearch/discovery_repository.go 7 9 77.78%
internal/store/postgres/lineage_repository.go 7 9 77.78%
core/asset/service.go 31 38 81.58%
core/asset/delete_asset_expr.go 30 40 75.0%
pkg/queryexpr/query_expr.go 36 56 64.29%
internal/store/postgres/asset_repository.go 65 99 65.66%
pkg/queryexpr/sql_expr.go 121 163 74.23%
pkg/queryexpr/es_expr.go 154 235 65.53%
Files with Coverage Reduction New Missed Lines %
internal/store/postgres/asset_repository.go 1 78.22%
Totals Coverage Status
Change from base Build 10502183645: -1.1%
Covered Lines: 6876
Relevant Lines: 8179

💛 - Coveralls

@luthfifahlevi luthfifahlevi requested a review from irainia August 21, 2024 07:41
internal/workermanager/worker_manager.go Show resolved Hide resolved
pkg/queryexpr/sql_expr.go Show resolved Hide resolved
pkg/query_expr/es_expr_test.go Outdated Show resolved Hide resolved
pkg/queryexpr/es_expr.go Outdated Show resolved Hide resolved
core/asset/delete_asset_expr.go Outdated Show resolved Hide resolved
core/asset/delete_asset_expr.go Outdated Show resolved Hide resolved
internal/server/v1beta1/asset.go Outdated Show resolved Hide resolved
internal/store/postgres/asset_repository.go Outdated Show resolved Hide resolved
internal/store/postgres/asset_repository.go Outdated Show resolved Hide resolved
internal/store/postgres/lineage_repository.go Show resolved Hide resolved
pkg/generichelper/generic_helper.go Outdated Show resolved Hide resolved
pkg/queryexpr/query_expr.go Show resolved Hide resolved
pkg/queryexpr/query_expr.go Outdated Show resolved Hide resolved
Copy link

@irainia irainia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small changes needed

internal/store/postgres/asset_repository.go Outdated Show resolved Hide resolved
@luthfifahlevi luthfifahlevi requested a review from irainia August 22, 2024 04:35
@luthfifahlevi luthfifahlevi requested a review from haveiss August 22, 2024 06:27
@luthfifahlevi luthfifahlevi merged commit 071df6f into main Aug 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants