Remove backward compatibility for manifest with artifact

[MichalPysik]

This commit removes support for manifests storing their data inside an artifact instead of using the recently introduced Manifest.data text field.

closes #1621

[lubosmj]

Have you considered returning manifest.data directly from api-app? We may want to omit redirects to content-app when the data is available. Thus, instead of doing this:

pulp_container/pulp_container/app/registry_api.py

Line 1083 in 81e5f11

return redirects.issue_manifest_redirect(manifest)

, we would run return Response(manifest.data).

[lubosmj]

I think tests are failing because of the caching machinery:

2024-06-03T10:51:37.9321965Z   File "/usr/local/lib/python3.9/site-packages/pulp_container/app/registry_api.py", line 1016, in get
2024-06-03T10:51:37.9322259Z     return self.handle_safe_method(request, path, pk)
2024-06-03T10:51:37.9323128Z   File "/usr/local/lib/python3.9/site-packages/pulpcore/cache/cache.py", line 171, in cached_function
2024-06-03T10:51:37.9323626Z     response = self.make_entry(key, bk, func, args, kwargs, self.default_expires_ttl)
2024-06-03T10:51:37.9324448Z   File "/usr/local/lib/python3.9/site-packages/pulpcore/cache/cache.py", line 220, in make_entry
2024-06-03T10:51:37.9324840Z     entry["content"] = response.content.decode("utf-8")
2024-06-03T10:51:37.9325657Z   File "/usr/local/lib/python3.9/site-packages/django/template/response.py", line 135, in content
2024-06-03T10:51:37.9325900Z     raise ContentNotRenderedError(
2024-06-03T10:51:37.9326730Z django.template.response.ContentNotRenderedError: The response content must be rendered before it can be accessed.

We started returning manifest.data from api-app which results in the said error. We may want to update the code on the pulpcore's side or overwrite the existing behaviour, where decode is always called on the TextField data, from the plugin itself.

# cache.py

     def make_entry():
          ...
          elif isinstance(response, HttpResponse):
            entry["type"] = "Response"
            if isinstance(response.data, str):
               # TODO: store response.content data
            else:
                entry["content"] = response.content.decode("utf-8")
        else:
            # We don't cache StreamResponses or errors
            return response

[MichalPysik]

I think tests are failing because of the caching machinery:

2024-06-03T10:51:37.9321965Z   File "/usr/local/lib/python3.9/site-packages/pulp_container/app/registry_api.py", line 1016, in get
2024-06-03T10:51:37.9322259Z     return self.handle_safe_method(request, path, pk)
2024-06-03T10:51:37.9323128Z   File "/usr/local/lib/python3.9/site-packages/pulpcore/cache/cache.py", line 171, in cached_function
2024-06-03T10:51:37.9323626Z     response = self.make_entry(key, bk, func, args, kwargs, self.default_expires_ttl)
2024-06-03T10:51:37.9324448Z   File "/usr/local/lib/python3.9/site-packages/pulpcore/cache/cache.py", line 220, in make_entry
2024-06-03T10:51:37.9324840Z     entry["content"] = response.content.decode("utf-8")
2024-06-03T10:51:37.9325657Z   File "/usr/local/lib/python3.9/site-packages/django/template/response.py", line 135, in content
2024-06-03T10:51:37.9325900Z     raise ContentNotRenderedError(
2024-06-03T10:51:37.9326730Z django.template.response.ContentNotRenderedError: The response content must be rendered before it can be accessed.

We started returning manifest.data from api-app which results in the said error. We may want to update the code on the pulpcore's side or overwrite the existing behaviour, where decode is always called on the TextField data, from the plugin itself.

# cache.py

     def make_entry():
          ...
          elif isinstance(response, HttpResponse):
            entry["type"] = "Response"
            if isinstance(response.data, str):
               # TODO: store response.content data
            else:
                entry["content"] = response.content.decode("utf-8")
        else:
            # We don't cache StreamResponses or errors
            return response

@lubosmj If I understand correctly, before we started returning manifest.data (which is text, not bytes), we used to return it in bytes (abstracting away the fact that it went through multiple additional functions to get to that point)? And if that is the case, would simply returning manifest.data.encode() fix the issue?

[lubosmj]

Oh, actually, the issue is that we cannot access response.content. Thus, the execution never reaches the decode call. Can you verify if calling "response.render()" inside the pulpcore's code will resolve the issue? Other than that, I think we should update the caching code as a whole to deal with HttpResponse properly. It is plausible that this code path was never executed. Or, we may think about returning something else from api-app...

[lubosmj]

Bear in mind that we would like to return headers as well, similarly to return web.Response(text=tag.tagged_manifest.data, headers=response_headers).

[MichalPysik]

@lubosmj What headers would you like to return in this case?

[lubosmj]

Do you mind reverting the following changes?

pulp_container/pulp_container/app/tasks/sync_stages.py

Line 336 in da8a70f

# FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288

pulp_container/pulp_container/app/tasks/sync_stages.py

Line 585 in da8a70f

# FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288

pulp_container/pulp_container/app/downloaders.py

Line 59 in da8a70f

# FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288

I think we can get rid of them now.

[lubosmj]

The work on this can be resumed now.

[stale]

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!