Skip to content

Conversation

@vbrodsky
Copy link
Contributor

@vbrodsky vbrodsky commented Jan 2, 2024

Story: https://labelbox.atlassian.net/browse/SDK-416
Corresponding API PR: https://github.com/Labelbox/intelligence/pull/18718

This Pull Request introduces the functionality to use global keys as a parameter for bulk metadata deletion, utilizing our UniqueId/GlobalKey approach.

To support backward compatibility we:

  • introduced a new DeleteDataRowMetadataV2 input type while keeping support for existing DeleteDataRowMetadata which will be deprecated in the future
  • additionally, I refactored the integration tests for bulk metadata operations by moving the bulk delete tests into its own file for clearer organization.

@vbrodsky vbrodsky requested review from a team, sqlboy and yamini-labelbox January 2, 2024 19:19
@vbrodsky vbrodsky force-pushed the VB/bulk-delete-metadata-by-gk_SDK-416 branch 2 times, most recently from 9923e04 to 5bd1033 Compare January 3, 2024 02:41
fields: List[SchemaId]


class DeleteDataRowMetadataV2(_CamelCaseMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to reuse the existing DeleteDataRowMetadata by adding the new field as an optional argument without creating DeleteDataRowMetadataV2? (i am unsure if this was already discussed and finalized to do it this way).

Copy link
Contributor

@yamini-labelbox yamini-labelbox left a comment

Choose a reason for hiding this comment

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

LGTM! Approved.

Added a minor comment


class _DeleteBatchDataRowMetadata(_CamelCaseMixin):
data_row_id: str
class _DeleteBatchDataRowMetadataV2(_CamelCaseMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I do not see any benefit of updating the name of this method to _DeleteBatchDataRowMetadataV2. It is a minor comment though, as this is not exposed to the user. Looks good to me otherwise! Thanks for the refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will rename back

@vbrodsky vbrodsky force-pushed the VB/bulk-delete-metadata-by-gk_SDK-416 branch from fefeaad to 13f2ef9 Compare January 4, 2024 03:45
@vbrodsky vbrodsky merged commit 632d64d into develop Jan 4, 2024
@vbrodsky vbrodsky deleted the VB/bulk-delete-metadata-by-gk_SDK-416 branch January 4, 2024 04:58
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.

3 participants