Manifest is now artifactless #1530

MichalPysik · 2024-03-01T17:10:38Z

Objects of the Manifest class no longer store the manifest data (raw text json data) in a linked artifact. Instead, a new TextField called 'data' was added to the Manifest class itself.

closes #1288

lubosmj · 2024-05-02T08:01:03Z

I am taking over this to address the pending comments. @MichalPysik, thanks for the effort!

Objects of the Manifest class no longer store the manifest data (raw binary json data) in a linked artifact. Instead, a new TextField called 'data' was added to the Manifest class itself. closes pulp#1288

ipanova · 2024-05-03T14:32:04Z

pulp_container/app/management/commands/container-handle-image-data.py

+
+    def init_manifest(self, manifest):
+        has_initialized_data = manifest.data is not None
+        if has_initialized_data:


if manifest.data: should suffice, no?

We check whether data was updated or not when returning from this method. So, we need to assess the state before updating via has_initialized_data.

has_initialized_data is not needed. you can check directly if manifest.data ( because there will whether be data or be none)

In addition to this, I generally do not understand why you are json-loading data and parse it again for labels?
If manifest.data is not none it means on upload/sync it was already parsed and necessary data populated.

has_initialized_data is not needed. you can check directly if manifest.data ( because there will whether be data or be none)

Actually, manifest.data is updated along the way in the code; we want to assess the initial state of the things.

what do you mean updated? manifest.data is its payload, it does not change

Couple of lines below the if condition, we update the data attribute of the manifest object:

manifest_data, raw_bytes_data = get_content_data(manifest_artifact) manifest.data = raw_bytes_data.decode("utf-8")

When returning from the method, we check if the object was updated (not has_initialized_data), if so, we run the DB migration.

Oh i see that now. But if has_initialized_data is true then there is no need to init_labels, annotations etc. so you need to update the logic there

pulp_container/app/management/commands/container-handle-image-data.py

ipanova · 2024-05-03T14:41:23Z

pulp_container/app/management/commands/container-handle-image-data.py

+        if has_initialized_data:
+            manifest_data = json.loads(manifest.data)
+        else:
+            manifest_artifact = manifest._artifacts.get()


replace with .first(), artifact might be missing

DoesNotExist is raised when there is no artifact. This is caught by with suppress(ObjectDoesNotExist, JSONDecodeError).

ipanova · 2024-05-06T08:35:55Z

pulp_container/app/management/commands/container-handle-image-data.py

+
+    def init_manifest(self, manifest):
+        has_initialized_data = manifest.data is not None
+        if has_initialized_data:


has_initialized_data is not needed. you can check directly if manifest.data ( because there will whether be data or be none)

ipanova · 2024-05-06T08:46:41Z

pulp_container/app/management/commands/container-handle-image-data.py

+
+    def init_manifest(self, manifest):
+        has_initialized_data = manifest.data is not None
+        if has_initialized_data:


In addition to this, I generally do not understand why you are json-loading data and parse it again for labels?
If manifest.data is not none it means on upload/sync it was already parsed and necessary data populated.

ipanova · 2024-05-06T08:47:18Z

pulp_container/app/management/commands/container-handle-image-data.py

@@ -28,47 +31,40 @@ class Command(BaseCommand):
    stored inside artifacts.

    This command reads data from the storage backend and populates the 'annotations', 'labels',
-    'is_bootable', and 'is_flatpak' fields on the Manifest model.
+    'is_bootable', and 'is_flatpak' fields on the Manifest model. Note that the Redis cache will


+ moves manifest artifact into DB

ipanova · 2024-05-06T08:48:22Z

pulp_container/app/migrations/0039_manifest_data.py

@@ -20,7 +21,7 @@ class Migration(migrations.Migration):
        migrations.AddField(
            model_name="manifest",
            name="data",
-            field=models.TextField(default=""),
+            field=models.TextField(null=True),


you should update respectively your queries from data=None to data__isnull=True/False

ipanova

Thanks for all the work, few comments that need addressing, approving ahead.

[noissue]

MichalPysik · 2024-05-06T17:39:14Z

Great job @lubosmj! Love to see this getting finally merged!

MichalPysik force-pushed the artifactless_manifest branch 2 times, most recently from f4b0ace to 504ac67 Compare March 4, 2024 12:42

lubosmj mentioned this pull request Mar 5, 2024

Distinguish between the nature of images #1532

Merged

MichalPysik force-pushed the artifactless_manifest branch 15 times, most recently from 795f281 to 4b59675 Compare March 11, 2024 12:52

MichalPysik force-pushed the artifactless_manifest branch 5 times, most recently from 47f0683 to 1304b4b Compare March 12, 2024 15:54

MichalPysik force-pushed the artifactless_manifest branch 7 times, most recently from 0481be2 to a134f49 Compare March 25, 2024 10:36

lubosmj marked this pull request as draft May 2, 2024 07:59

lubosmj self-assigned this May 2, 2024

lubosmj force-pushed the artifactless_manifest branch from a4136fc to 04266d8 Compare May 2, 2024 12:26

github-actions bot added the multi-commit label May 2, 2024

Manifest is now artifactless

6ea6a91

Objects of the Manifest class no longer store the manifest data (raw binary json data) in a linked artifact. Instead, a new TextField called 'data' was added to the Manifest class itself. closes pulp#1288

git-hyagi force-pushed the artifactless_manifest branch from ede14f3 to 6ea6a91 Compare May 2, 2024 18:16

github-actions bot added multi-commit and removed multi-commit labels May 2, 2024

lubosmj force-pushed the artifactless_manifest branch 2 times, most recently from 35bd9b7 to 76d6769 Compare May 3, 2024 14:38

ipanova reviewed May 3, 2024

View reviewed changes

lubosmj force-pushed the artifactless_manifest branch 2 times, most recently from d36f8a0 to e499471 Compare May 3, 2024 18:04

lubosmj marked this pull request as ready for review May 3, 2024 18:05

ipanova reviewed May 6, 2024

View reviewed changes

ipanova mentioned this pull request May 6, 2024

Test how much time takes the handle-image django-admin command with OSP content #1573

Closed

ipanova approved these changes May 6, 2024

View reviewed changes

lubosmj force-pushed the artifactless_manifest branch 3 times, most recently from ec107c3 to 02e55c7 Compare May 6, 2024 11:27

lubosmj marked this pull request as draft May 6, 2024 12:15

lubosmj force-pushed the artifactless_manifest branch 2 times, most recently from b0c9b12 to b16c6ff Compare May 6, 2024 12:45

Address pending comments and clean up the code

067f292

[noissue]

lubosmj force-pushed the artifactless_manifest branch from b16c6ff to 067f292 Compare May 6, 2024 12:47

lubosmj marked this pull request as ready for review May 6, 2024 12:50

lubosmj merged commit 13bf171 into pulp:main May 6, 2024
16 checks passed

git-hyagi mentioned this pull request Jun 19, 2024

Validate manifest raises wrong digest in case this is a schema1 type #1130

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manifest is now artifactless #1530

Manifest is now artifactless #1530

MichalPysik commented Mar 1, 2024 •

edited

Loading

lubosmj commented May 2, 2024

ipanova May 3, 2024

lubosmj May 3, 2024

ipanova May 6, 2024

ipanova May 6, 2024

lubosmj May 6, 2024 •

edited

Loading

ipanova May 6, 2024 •

edited

Loading

lubosmj May 6, 2024

ipanova May 6, 2024

ipanova May 3, 2024

lubosmj May 3, 2024

ipanova May 6, 2024

ipanova May 6, 2024

ipanova May 6, 2024

ipanova May 6, 2024 •

edited

Loading

ipanova left a comment

MichalPysik commented May 6, 2024

Manifest is now artifactless #1530

Manifest is now artifactless #1530

Conversation

MichalPysik commented Mar 1, 2024 • edited Loading

lubosmj commented May 2, 2024

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

lubosmj May 6, 2024 • edited Loading

Choose a reason for hiding this comment

ipanova May 6, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ipanova May 6, 2024 • edited Loading

Choose a reason for hiding this comment

ipanova left a comment

Choose a reason for hiding this comment

MichalPysik commented May 6, 2024

MichalPysik commented Mar 1, 2024 •

edited

Loading

lubosmj May 6, 2024 •

edited

Loading

ipanova May 6, 2024 •

edited

Loading

ipanova May 6, 2024 •

edited

Loading