Add number of extracted, loaded, and failed resources in batch responses #693

Jasopaum · 2021-10-25T14:07:24Z

TODO: front

Fixes

Fixes #668 by @MiskoG

Description

Back: add a progressions field to Batch
Front: display this information in the "ETL view"

Technical details

If the batch is running, the progression is fetched from redis. Otherwise, it's persisted and fetched from the DB

Definition of Done

I followed the Arkhn Code Book (I swear!).
I have written tests for the code I added or updated.
I have updated the documentation according to my changes.
I have updated the deployment configuration if needed.

This field is a SerializerMethodField that fetches the progression for each resource from redis

tevariou

can you update river-schema.yml as well?

django/river/api/serializers/serializers.py

simonvadee

IMO it's best to use a dedicated table for Progression. having a jsonfield with a custom structure in the Batch table doesn't make sense in a relational db, especially if we want to make queries involving a progression counter by resource_id.
@tevariou what's your opinion ?

tevariou · 2021-10-25T16:46:52Z

IMO it's best to use a dedicated table for Progression. having a jsonfield with a custom structure in the Batch table doesn't make sense in a relational db, especially if we want to make queries involving a progression counter by resource_id. @tevariou what's your opinion ?

I agree it may be more practical to have a dedicated table for filtering

Jasopaum · 2021-10-25T17:03:41Z

Also, @tevariou, @simonvadee, do you know if there is an easy way to know the number of resources that have been updated compared to inserted?

simonvadee · 2021-10-26T09:19:46Z

do you know if there is an easy way to know the number of resources that have been updated compared to inserted?

good question: there isn't. In loader, we use dao.update() whether we want to insert or update a resource, and we always increment the loaded_counters. We could make a distinction between inserted and updated depending on the return value of dao.update() maybe ? I encourage you to create an issue.

tevariou · 2021-10-26T12:29:21Z

django/river/api/serializers/serializers.py

@@ -11,6 +15,7 @@ class Meta:

 class BatchSerializer(serializers.ModelSerializer):
    errors = ErrorSerializer(many=True, read_only=True)
+    progressions = serializers.SerializerMethodField()


Since we created a model for this field, maybe we need to do something like that https://www.django-rest-framework.org/api-guide/relations/#nested-relationships

But it adds complexity (and I know Valentin didn't like nested serializers).. maybe it would be better to create a new route

Right, but I didn't know how to combine a SerializerMethodField and model Serializers... Do you know how we could do that?

Changed to stop using SerializerMethodField

simonvadee

only small comments 💯

simonvadee · 2021-11-08T13:21:25Z

django/river/api/serializers/serializers.py

@@ -21,6 +42,40 @@ class Meta:
            "completed_at": {"allow_null": True},
        }

+    def get_progressions(self, obj) -> List[Tuple[str, Dict]]:


I think this is not used anymore: you use ProgressionSerializer and ResourceForProgressionSerializer instead

simonvadee · 2021-11-08T13:27:00Z

tests/river/e2e/api/test_batch.py

+    progression_factory.create(batch=batch, resource=r1)
+    progression_factory.create(batch=batch, resource=r2)


can you define extracted/loaded/failed here instead of directly in the factory ? easier to understand

simonvadee · 2021-11-08T13:27:32Z

tests/river/e2e/api/test_batch.py

+    progression_factory.create(batch=batch, resource=r1)
+    progression_factory.create(batch=batch, resource=r2)


simonvadee · 2021-11-08T13:27:39Z

tests/river/e2e/api/test_batch.py

+
+    response_get = api_client.get(url)
+    assert response_get.json()["canceled_at"] is not None
+    print(response_get.json()["progressions"])


simonvadee · 2021-11-08T13:28:12Z

tests/river/factories.py

+    extracted = 100
+    loaded = 50
+    failed = None


IMO: should be removed

Jasopaum added 6 commits October 25, 2021 12:00

feat(river): add progressions field in batch serializer

10a17b7

This field is a SerializerMethodField that fetches the progression for each resource from redis

test(river): test Batch's progressions field

1c8995d

feat(river): add progressions field to Batch

6d98543

test(river): persist progressions on batch cancellation

8d50ed0

feat(river): turn progressions field of Batch to list of lists

c56feca

test(river): persist progressions on batch end

613a1e7

Jasopaum changed the title ~~Display number of extracted, loaded, and failed resources in ETL view~~ [WIP] Display number of extracted, loaded, and failed resources in ETL view Oct 25, 2021

Jasopaum requested review from tevariou and simonvadee and removed request for tevariou October 25, 2021 14:19

tevariou suggested changes Oct 25, 2021

View reviewed changes

django/river/api/serializers/serializers.py Outdated Show resolved Hide resolved

django/river/api/serializers/serializers.py Show resolved Hide resolved

Jasopaum added 3 commits October 25, 2021 17:19

test(river): fix test_abort with progressions

91fcda0

feat(river): add doc to get_progressions on Batch serializer

9646835

feat(river): make Batch.progressions read only

ff1bf72

Jasopaum requested a review from tevariou October 25, 2021 15:26

simonvadee reviewed Oct 25, 2021

View reviewed changes

Jasopaum added 3 commits October 26, 2021 12:41

feat(river): persist Progressions in table instead of json column

300d0ed

bump(river): bump drf-spectacular to 0.17.3

a84fb3b

feat(river): update openapi schema with progressions

ad84efc

Jasopaum requested a review from simonvadee October 26, 2021 10:46

Jasopaum changed the title ~~[WIP] Display number of extracted, loaded, and failed resources in ETL view~~ Display number of extracted, loaded, and failed resources in ETL view Oct 26, 2021

tevariou reviewed Oct 26, 2021

View reviewed changes

Jasopaum changed the title ~~Display number of extracted, loaded, and failed resources in ETL view~~ Add number of extracted, loaded, and failed resources in batch responses Oct 26, 2021

feat(river): persist progressions with topicleaner

b6981ee

Jasopaum force-pushed the jason/display-extract-load branch from 9d1791c to b6981ee Compare October 27, 2021 10:07

simonvadee reviewed Nov 8, 2021

View reviewed changes

Jasopaum closed this Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add number of extracted, loaded, and failed resources in batch responses #693

Add number of extracted, loaded, and failed resources in batch responses #693

Jasopaum commented Oct 25, 2021 •

edited

Loading

tevariou left a comment

simonvadee left a comment •

edited

Loading

tevariou commented Oct 25, 2021

Jasopaum commented Oct 25, 2021

simonvadee commented Oct 26, 2021

tevariou Oct 26, 2021

Jasopaum Oct 26, 2021

Jasopaum Oct 26, 2021

simonvadee left a comment

simonvadee Nov 8, 2021

simonvadee Nov 8, 2021

simonvadee Nov 8, 2021

simonvadee Nov 8, 2021

simonvadee Nov 8, 2021

		progression_factory.create(batch=batch, resource=r1)
		progression_factory.create(batch=batch, resource=r2)

Add number of extracted, loaded, and failed resources in batch responses #693

Add number of extracted, loaded, and failed resources in batch responses #693

Conversation

Jasopaum commented Oct 25, 2021 • edited Loading

Fixes

Description

Technical details

Definition of Done

tevariou left a comment

Choose a reason for hiding this comment

simonvadee left a comment • edited Loading

Choose a reason for hiding this comment

tevariou commented Oct 25, 2021

Jasopaum commented Oct 25, 2021

simonvadee commented Oct 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonvadee left a 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

Jasopaum commented Oct 25, 2021 •

edited

Loading

simonvadee left a comment •

edited

Loading