feat(deletes): add run commands (list, show, rollback) to datahub ingest #2960

gabe-lyons · 2021-07-27T17:12:35Z

Adds three new commands to datahub ingest:

datahub ingest list-runs - lists recent runs ingested to datahub
datahub ingest show-run - describes an individual ingestion run
datahub ingest rollback-run - reverts the changes made during an ingestion run

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable)

hsheth2 · 2021-07-27T23:35:15Z

metadata-ingestion/examples/mce_files/bootstrap_mce2.json

@@ -0,0 +1,1974 @@
+[
+  {
+    "auditHeader": null,


What's the difference between this and the original bootstrap file?

This was for testing-- will remove.

hsheth2 · 2021-07-27T23:45:07Z

metadata-ingestion/src/datahub/cli/ingest_cli.py

+        click.echo(f"Received error, please check your {CONDENSED_DATAHUB_CONFIG_PATH}")
+        click.echo()
+        click.echo(response_json)
+        exit()


this code is awfully repetitive

hsheth2 · 2021-07-27T23:46:49Z

metadata-ingestion/src/datahub/cli/ingest_cli.py

+            gms_config = config.get("gms")
+            if not isinstance(gms_config, dict) or gms_config.get("server") is None:
+                print_datahub_env_format_guide()
+                exit()


We should use pydantic for all of this validation logic

Cool- will look into that!

hsheth2 · 2021-07-27T23:52:19Z

metadata-ingestion/src/datahub/ingestion/extractor/mce_extractor.py

 from datahub.metadata.schema_classes import UsageAggregationClass


+def current_milli_time():
+    return round(t.time() * 1000)


mce_builder.py get_sys_time does essentially the same thing - could we just use that?

hsheth2 · 2021-07-27T23:56:57Z

metadata-ingestion/src/datahub/ingestion/extractor/mce_extractor.py

+
+            system_metadata = SystemMetadata(
+                lastObserved=current_milli_time(), runId=self.ctx.run_id
+            )


note: the extractor runs before the transformers, and so any MCEs created by a transformer won't get the system metadata :(

This isn't an immediate issue since our current transformers only modify MCEs and don't create any, but it will definitely come up.

Ok- good tip. Any suggestions on how to best circumvent this?

We'll need to split the extractor up into (1) extracting RecordEnvelopes from work units and (2) running validation + checks. (2) can probably be implemented as a transformer that always runs

hsheth2 · 2021-07-27T23:57:37Z

metadata-ingestion/src/datahub/ingestion/extractor/mce_extractor.py

+                lastObserved=current_milli_time(), runId=self.ctx.run_id
+            )
+
+            workunit.mce.systemMetadata = system_metadata


also note: the run_id config param of the pipeline will eventually need to be changed to run_id_prefix

hsheth2 · 2021-07-27T23:58:39Z

metadata-ingestion/tests/integration/hive/test_hive.py


+
+@freeze_time(FROZEN_TIME)


im surprised that this one needed a freeze_time given that it already had mock_time

Me too- this was definitely necessary though.

jjoyce0510 · 2021-07-28T20:49:59Z

gms/client/src/main/java/com/linkedin/entity/client/EntityClient.java

-        builder.name("ingest");
-        builder.addParam(entityFieldDef, entity);
+      SystemMetadata generatedSystemMetadata = new SystemMetadata();
+      generatedSystemMetadata.setLastObserved(System.currentTimeMillis());


just curious - why isn't GMS itself adding this?

(Seems like clients could mess this up if they all need to remember to set it)

jjoyce0510 · 2021-07-28T20:52:27Z

gms/impl/src/main/java/com/linkedin/metadata/resources/entity/EntityResource.java

+
+    if (systemMetadata == null) {
+      SystemMetadata generatedSystemMetadata = new SystemMetadata();
+      generatedSystemMetadata.setLastObserved(System.currentTimeMillis());


Oh got it - so client can optionally overwrite this. It still may make sense to keep last observed consistent by overriding this on the GMS side.. thoughts?

good call- i already have logic to handle system metadata not being set in gms. Updating so the client sends a null system metadata if it doesn't receive one.

jjoyce0510 · 2021-07-28T20:53:30Z

gms/impl/src/main/java/com/linkedin/metadata/resources/entity/EntityResource.java

      return null;
    });
  }

  @Action(name = ACTION_BATCH_INGEST)
  @Nonnull
-  public Task<Void> batchIngest(@ActionParam(PARAM_ENTITIES) @Nonnull Entity[] entities) throws URISyntaxException {
+  public Task<Void> batchIngest(
+      @ActionParam(PARAM_ENTITIES) @Nonnull Entity[] entities,


Can we validate that the list of entities is the same size as the list of system metadata?

will add 👍

what if systemMetadataList is null?

@gabe-lyons

Good call- I'll generate system metadata if not provided

jjoyce0510 · 2021-07-28T20:54:00Z

gms/impl/src/main/java/com/linkedin/metadata/resources/entity/EntityResource.java

@@ -198,6 +219,30 @@
    return RestliUtils.toTask(() -> new StringArray(_searchService.getBrowsePaths(urnToEntityName(urn), urn)));
  }

+  /*
+  Used to enable writes in GMS after data migration is complete


Misplaced comment

jjoyce0510 · 2021-07-28T20:55:01Z

gms/impl/src/main/java/com/linkedin/metadata/resources/entity/EntityResource.java

+    return RestliUtils.toTask(() -> {
+      RollbackResponse response = new RollbackResponse();
+
+      EbeanEntityService.RollbackRunResult result = _entityService.rollbackUrn(urn);


Is this just hard deleting this urn? If so, why not call it "deleteEntity"

ah I like that, will update

jjoyce0510 · 2021-07-28T20:57:09Z

gms/impl/src/main/java/com/linkedin/metadata/resources/entity/EntityResource.java

+
+      response.setAspectsAffected(deletedRows.size());
+      response.setEntitiesAffected(deletedRows.stream().filter(row -> row.isKeyAspect()).count());
+      response.setAspectRowSummaries(


This is nice!

shirshanka · 2021-07-30T01:27:06Z

docs/how/delete-metadata.md

+datahub delete --urn "<my urn>"
+```
+
+_Note: make sure you surround your urn with quotes! If you do not include the quotes, your terminal my misinterpret the command._


nit: typo s/my/may

… RunDeleteApi

shirshanka

LGTM

gabe-lyons added 17 commits July 26, 2021 09:23

updating mce format

d0a0b3b

adding delete api

08f0f39

checkpointing mysql solution

5cbd7b8

checkpointing with broken elastic situation

b556ea4

finished up deletes API

f16a351

fixing large page size descriptions

d13ec61

fixing tests

84002a6

trying to checkpoint python tests

7aca10e

correctness of rollback counts

26d736e

merge lookml

236552d

fixing final tests

1593a11

finishing up rollbacks

58e20e5

fixing tabulate import

d950554

fixing integration tests

4a9a7ad

updating idl and tests

3ff68b5

lint fixes

2d8e583

updating tests and cli

1f6f25b

hsheth2 requested changes Jul 28, 2021

View reviewed changes

gabe-lyons added 6 commits July 27, 2021 17:01

updated tests

e09ae46

removing boostrap2

b91d2f0

using get_sys_time

2ef7dff

updating tests and add rollback of individual urn

da69080

updating restspec

a4092c0

adding delete urn capabilities

c461e74

jjoyce0510 reviewed Jul 28, 2021

View reviewed changes

gabe-lyons added 8 commits July 29, 2021 18:25

fixing large page size descriptions

15f710c

fixing tests

a219951

trying to checkpoint python tests

9acf018

correctness of rollback counts

f4b1f3c

merge lookml

ed560a7

fixing final tests

f56d437

finishing up rollbacks

10edabd

fixing tabulate import

3b4e619

shirshanka reviewed Jul 30, 2021

View reviewed changes

gabe-lyons added 15 commits July 29, 2021 18:29

fixing integration tests

4f236e1

updating idl and tests

06cdb95

lint fixes

5d95c43

updating tests and cli

5de1889

updated tests

ef65ad9

removing boostrap2

ba58f32

using get_sys_time

353334b

updating tests and add rollback of individual urn

f3d7783

updating restspec

25f7ac6

adding delete urn capabilities

495865a

using pydantic for validation

d7a362e

refactoring

9775d04

fixing checkstyle

20d5e66

changing response + generating keys a level up

b7f246c

adding documentation

2a47bab

gabe-lyons force-pushed the RunDeleteApi branch from 5699f98 to 2a47bab Compare July 30, 2021 01:32

gabe-lyons added 3 commits July 29, 2021 18:38

updating entity doc

df8a68c

Merge branch 'RunDeleteApi' of github.com:acryldata/datahub-fork into…

8bc227e

… RunDeleteApi

fixing docs

7f2191c

shirshanka approved these changes Jul 30, 2021

View reviewed changes

shirshanka merged commit aa253f5 into datahub-project:master Jul 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(deletes): add run commands (list, show, rollback) to datahub ingest #2960

feat(deletes): add run commands (list, show, rollback) to datahub ingest #2960

gabe-lyons commented Jul 27, 2021

hsheth2 Jul 27, 2021

gabe-lyons Jul 28, 2021

hsheth2 Jul 27, 2021

hsheth2 Jul 27, 2021

gabe-lyons Jul 28, 2021

gabe-lyons Jul 29, 2021

hsheth2 Jul 27, 2021

gabe-lyons Jul 28, 2021

hsheth2 Jul 27, 2021

gabe-lyons Jul 28, 2021

hsheth2 Jul 28, 2021

hsheth2 Jul 27, 2021

hsheth2 Jul 27, 2021

gabe-lyons Jul 28, 2021

jjoyce0510 Jul 28, 2021

jjoyce0510 Jul 28, 2021

jjoyce0510 Jul 28, 2021

gabe-lyons Jul 29, 2021

jjoyce0510 Jul 28, 2021

gabe-lyons Jul 29, 2021

gabe-lyons Jul 29, 2021

dexter-mh-lee Jul 29, 2021

dexter-mh-lee Jul 29, 2021

gabe-lyons Jul 29, 2021

jjoyce0510 Jul 28, 2021

gabe-lyons Jul 29, 2021

jjoyce0510 Jul 28, 2021 •

edited

Loading

gabe-lyons Jul 29, 2021

jjoyce0510 Jul 28, 2021

gabe-lyons Jul 29, 2021

shirshanka Jul 30, 2021

shirshanka left a comment



		@freeze_time(FROZEN_TIME)

feat(deletes): add run commands (list, show, rollback) to datahub ingest #2960

feat(deletes): add run commands (list, show, rollback) to datahub ingest #2960

Conversation

gabe-lyons commented Jul 27, 2021

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjoyce0510 Jul 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shirshanka left a comment

Choose a reason for hiding this comment

jjoyce0510 Jul 28, 2021 •

edited

Loading