Handles a `PushRepository.DoesNotExist` exception #1727

git-hyagi · 2024-08-01T19:35:57Z

lubosmj · 2024-08-02T08:12:51Z

pulp_container/app/registry_api.py

+                    message='Cannot create "' + path + '" push repository, another repository with '
+                    "the same name already exists.",


Too much information. Let's stick to raising just RepositoryInvalid(name=path, message="Repository is read-only.").

lubosmj · 2024-08-02T08:13:52Z

docs/user/guides/push-image.md

+    remote content via the Pulp API. Trying to push a content with the same name of an existing
+    "regular" repository will fail.


Suggested change

remote content via the Pulp API. Trying to push a content with the same name of an existing

"regular" repository will fail.

remote content via the Pulp API. Trying to push content with the same name as an existing

"regular" repository will fail.

lubosmj · 2024-08-02T08:15:09Z

CHANGES/1712.bugfix

+Resolved an issue that caused an HTTP 500 error when attempting to push an image with the same name
+as an existing regular repository.


Suggested change

Resolved an issue that caused an HTTP 500 error when attempting to push an image with the same name

as an existing regular repository.

Fixed an HTTP 500 error returned when pushing an image with the same name as the name of an existing read-only repository.

lubosmj · 2024-08-02T08:28:44Z

pulp_container/app/registry_api.py

            distribution = serializers.ContainerDistributionSerializer.get_or_create(
                {"base_path": path, "name": path}, {"repository": get_url(repository)}
            )


We also need to catch an exception here.

pulp file distribution create --name test/mygroup/ubuntu --base-path test/mygroup/ubuntu podman push localhost:5001/test/mygroup/ubuntu --tls-verify=false

... [pulp] | raise ValidationError(self.errors) [pulp] | rest_framework.exceptions.ValidationError: {'name': [ErrorDetail(string='This field must be unique.', code='unique')], 'base_path': [ErrorDetail(string='This field must be unique.', code='unique')]} [pulp] | [pulp] | During handling of the above exception, another exception occurred: [pulp] | [pulp] | Traceback (most recent call last): [pulp] | File "/usr/local/lib/python3.9/site-packages/django/core/handlers/exception.py", line 55, in inner [pulp] | response = get_response(request) [pulp] | File "/usr/local/lib/python3.9/site-packages/django/core/handlers/base.py", line 197, in _get_response [pulp] | response = wrapped_callback(request, *callback_args, **callback_kwargs) [pulp] | File "/usr/local/lib/python3.9/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view [pulp] | return view_func(*args, **kwargs) [pulp] | File "/usr/local/lib/python3.9/site-packages/rest_framework/viewsets.py", line 124, in view [pulp] | return self.dispatch(request, *args, **kwargs) [pulp] | File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 509, in dispatch [pulp] | response = self.handle_exception(exc) [pulp] | File "/src/pulp_container/pulp_container/app/registry_api.py", line 279, in handle_exception [pulp] | response = super().handle_exception(exc) [pulp] | File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 469, in handle_exception [pulp] | self.raise_uncaught_exception(exc) [pulp] | File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception [pulp] | raise exc [pulp] | File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 506, in dispatch [pulp] | response = handler(request, *args, **kwargs) [pulp] | File "/src/pulp_container/pulp_container/app/registry_api.py", line 773, in create [pulp] | _, repository = self.get_dr_push(request, path, create=True) [pulp] | File "/src/pulp_container/pulp_container/app/registry_api.py", line 373, in get_dr_push [pulp] | distribution, repository = self.create_dr(path, request) [pulp] | File "/src/pulp_container/pulp_container/app/registry_api.py", line 405, in create_dr [pulp] | distribution = serializers.ContainerDistributionSerializer.get_or_create( [pulp] | File "/src/pulpcore/pulpcore/app/serializers/base.py", line 381, in get_or_create [pulp] | result = cls.Meta.model.objects.get(**natural_key) [pulp] | File "/usr/local/lib/python3.9/site-packages/django/db/models/manager.py", line 87, in manager_method [pulp] | return getattr(self.get_queryset(), name)(*args, **kwargs) [pulp] | File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 637, in get [pulp] | raise self.model.DoesNotExist( [pulp] | pulp_container.app.models.ContainerDistribution.DoesNotExist: ContainerDistribution matching query does not exist.

Maybe, we should start monitoring both get_or_create calls. django.core.exceptions.ObjectDoesNotExist would do us a favour.

lubosmj · 2024-08-02T20:39:32Z

pulp_container/app/registry_api.py

+                    {"base_path": path, "name": path}, {"repository": get_url(repository)}
+                )
+            except models.ContainerDistribution.DoesNotExist:
+                raise InvalidRequest(message="ContainerDistribution does not exist.")


I guess we can raise again RepositoryInvalid. The distribution exists. But, it has a different type than anticipated and I have no idea what error message we should pass to the client...

lubosmj · 2024-08-05T13:18:31Z

pulp_container/app/registry_api.py

+                    name=path,
+                    message="Cannot push to "
+                    + path
+                    + ". Another plugin with the same distribution name already exists.",


What if there is a pulp-container pull-through distribution that has the same name? Is it still another plugin? 😈

Good catch!

lubosmj · 2024-08-05T16:22:36Z

pulp_container/app/registry_api.py

+            try:
+                repository = serializers.ContainerPushRepositorySerializer.get_or_create(
+                    {"name": path}
+                )
+            except models.ContainerPushRepository.DoesNotExist:
+                raise RepositoryInvalid(name=path, message="Repository is read-only.")
+
+            try:
+                distribution = serializers.ContainerDistributionSerializer.get_or_create(
+                    {"base_path": path, "name": path}, {"repository": get_url(repository)}
+                )
+            except models.ContainerDistribution.DoesNotExist:
+                raise RepositoryInvalid(name=path, message="Repository is ready-only.")


Suggested change

try:

repository = serializers.ContainerPushRepositorySerializer.get_or_create(

{"name": path}

)

except models.ContainerPushRepository.DoesNotExist:

raise RepositoryInvalid(name=path, message="Repository is read-only.")

try:

distribution = serializers.ContainerDistributionSerializer.get_or_create(

{"base_path": path, "name": path}, {"repository": get_url(repository)}

)

except models.ContainerDistribution.DoesNotExist:

raise RepositoryInvalid(name=path, message="Repository is ready-only.")

try:

repository = serializers.ContainerPushRepositorySerializer.get_or_create(

{"name": path}

)

distribution = serializers.ContainerDistributionSerializer.get_or_create(

{"base_path": path, "name": path}, {"repository": get_url(repository)}

)

except ObjectDoesNotExist:

raise RepositoryInvalid(name=path, message="Repository is ready-only.")

🤦‍♂️

hum... actually, it seems like it does not work:

... File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 637, in get raise self.model.DoesNotExist( pulp_container.app.models.ContainerDistribution.DoesNotExist: ContainerDistribution matching query does not exist. ... File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 637, in get raise self.model.DoesNotExist( pulp_container.app.models.ContainerPullThroughRemote.DoesNotExist: ContainerPullThroughRemote matching query does not exist.

the django ObjectDoesNotExist exception did not catch the ContainerDistribution.DoesNotExist and/or ContainerPullThroughRemote.DoesNotExist exceptions

nvm, the above exception is not related with the proposed change.

We opened a new issue (#1729) to investigate it.

fixes: pulp#1712

lubosmj

Simple and straightforward! 🐁

lubosmj requested changes Aug 2, 2024

View reviewed changes

git-hyagi force-pushed the push-repo-does-not-exists-error branch from 92876c3 to 2691a3b Compare August 2, 2024 18:28

lubosmj reviewed Aug 2, 2024

View reviewed changes

git-hyagi force-pushed the push-repo-does-not-exists-error branch from 2691a3b to 65461d7 Compare August 5, 2024 12:36

lubosmj reviewed Aug 5, 2024

View reviewed changes

git-hyagi force-pushed the push-repo-does-not-exists-error branch from 65461d7 to b4501fa Compare August 5, 2024 15:06

lubosmj reviewed Aug 5, 2024

View reviewed changes

git-hyagi mentioned this pull request Aug 5, 2024

Disallow removing pull-through remotes when referenced from a pull-through distribution #1729

Closed

Handles a PushRepository.DoesNotExist exception

3d2e657

fixes: pulp#1712

git-hyagi force-pushed the push-repo-does-not-exists-error branch from b4501fa to 3d2e657 Compare August 5, 2024 20:21

lubosmj approved these changes Aug 6, 2024

View reviewed changes

git-hyagi merged commit aafa1c8 into pulp:main Aug 7, 2024
10 checks passed

git-hyagi deleted the push-repo-does-not-exists-error branch August 7, 2024 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handles a `PushRepository.DoesNotExist` exception #1727

Handles a `PushRepository.DoesNotExist` exception #1727

git-hyagi commented Aug 1, 2024

lubosmj Aug 2, 2024

lubosmj Aug 2, 2024

lubosmj Aug 2, 2024 •

edited

Loading

lubosmj Aug 2, 2024 •

edited

Loading

lubosmj Aug 2, 2024 •

edited

Loading

lubosmj Aug 5, 2024

git-hyagi Aug 5, 2024

lubosmj Aug 5, 2024

git-hyagi Aug 5, 2024

git-hyagi Aug 5, 2024

git-hyagi Aug 5, 2024

lubosmj left a comment

		message='Cannot create "' + path + '" push repository, another repository with '
		"the same name already exists.",

		remote content via the Pulp API. Trying to push a content with the same name of an existing
		"regular" repository will fail.

		Resolved an issue that caused an HTTP 500 error when attempting to push an image with the same name
		as an existing regular repository.

	Resolved an issue that caused an HTTP 500 error when attempting to push an image with the same name
	as an existing regular repository.
	Fixed an HTTP 500 error returned when pushing an image with the same name as the name of an existing read-only repository.

Handles a PushRepository.DoesNotExist exception #1727

Handles a PushRepository.DoesNotExist exception #1727

Conversation

git-hyagi commented Aug 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lubosmj Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

lubosmj Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

lubosmj Aug 2, 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

lubosmj left a comment

Choose a reason for hiding this comment

Handles a `PushRepository.DoesNotExist` exception #1727

Handles a `PushRepository.DoesNotExist` exception #1727

lubosmj Aug 2, 2024 •

edited

Loading

lubosmj Aug 2, 2024 •

edited

Loading

lubosmj Aug 2, 2024 •

edited

Loading