Project

Profile

Help

Task #7908

closed

Make sure all exceptions live in pulpcore.plugin.exceptions

Added by ggainey over 3 years ago. Updated over 3 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Low
Assignee:
-
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
Groomed:
No
Sprint Candidate:
No
Tags:
Sprint:
Sprint 86
Quarter:

Description

See discussion at https://github.com/pulp/pulpcore/pull/1027#discussion_r524551412

This will require a deprecation cycle, with a given exception living "in two places" for one release. Once this issue is in MODIFIED, open a new issue to complete the move.

Actions #1

Updated by daviddavis over 3 years ago

  • Sprint/Milestone set to 3.9.0
Actions #2

Updated by fao89 over 3 years ago

  • Tracker changed from Issue to Task
  • % Done set to 0
  • Severity deleted (2. Medium)
  • Triaged deleted (No)
  • Sprint set to Sprint 86
Actions #3

Updated by ggainey over 3 years ago

We'll want to document this in the plugin-writer's guide and at CHANGES/plugin_api/7908.feature, so that plugin-authors will know to look for exceptions available at pulpcore.plugin.exceptions

Actions #4

Updated by ggainey over 3 years ago

The only two Exception-definitions I have found outside of pulpcore.exceptions:

  • pulpcore/content/handler.py:class ArtifactNotFound(Exception):
  • pulpcore/app/models/content.py:class UnsupportedDigestValidationError(Exception):

pulpcore.content.handler.ArtifactNotFound appears to not be used anywhere.

There exists a pulp_container.app.registry.ArtifactNotFound(Exception), whichis unrelated (other than the name)

Proposal:

  • Delete pulpcore.content.handler.ArtifactNotFound as dead code
  • Create a pulpcore.exceptions.validation.UnsupportedDigestValidationError
  • Make that UnsupportedDigestValidationError available to pulpcore.plugin.exceptions
  • Update all of pulpcore to find UnsupportedDigestValidationError from its new location
  • Mark pulpcore.app.models.UnsupportedDigestValidationError as deprecated via announcement in CHANGES

In 3.10, remove pulpcore.app.models.UnsupportedDigestValidationError

Actions #5

Updated by bmbouter over 3 years ago

Since pulpcore.app.models.UnsupportedDigestValidationError is not part of the pulpcore.plugin package, we don't need to put it through the deprecation cycle. Except for this 1 change I think the plan you outline is what should be done. I don't think this change will require any deprecation at all actually.

Actions #6

Updated by daviddavis over 3 years ago

One thing that is unclear to me: is pulpcore.plugin.models.UnsupportedDigestValidationError remaining as is? I don't see anything in the plan about that and it would be kind of redundant with the step to "Make that UnsupportedDigestValidationError available to pulpcore.plugin.exceptions".

Actions #7

Updated by ggainey over 3 years ago

daviddavis wrote:

One thing that is unclear to me: is pulpcore.plugin.models.UnsupportedDigestValidationError remaining as is? I don't see anything in the plan about that and it would be kind of redundant with the step to "Make that UnsupportedDigestValidationError available to pulpcore.plugin.exceptions".

OK, so there are two needs here. One is to keep all the exceptions in one place, and the other is to make them accessible 'outside' of pulpcore itself. To satisfy these, UnsupportedDigestValidationError needs to move to pulpcore.plugin.exceptions.

The only problem with "just moving it", is if there exists non-pulpcore code that depends on finding it at pulpcore.plugin.models, they'll break. To be correct, we should :

  • make it available at pulpcore.plugins.exceptions,
  • leave it available at pulpcore.plugins.models, imported from its new location
  • announce the pulpcore.plugin.models one is deprecated, and then
  • remove that one in 3.10.

Whew. Does that match everyone's expectations?

Actions #8

Updated by daviddavis over 3 years ago

Whew. Does that match everyone's expectations?

Indeed it does.

Actions #9

Updated by pulpbot over 3 years ago

  • Status changed from NEW to POST

Added by ggainey over 3 years ago

Revision 86fd56f4 | View on GitHub

Moved UnsupportedDigestValidationError to new home.

It now lives in pulpcore.plugin.exceptions.

closes #7908

Actions #10

Updated by ggainey over 3 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100
Actions #11

Updated by pulpbot over 3 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Also available in: Atom PDF