[Pulp-dev] Solving the "callback problem" ... aka how pulpcore will stop finalizing RepositoryVersion

[Brian Bouterse]

Thank you for writing!

On Tue, Nov 5, 2019 at 4:29 PM Matthias Dellweg <dellweg at atix.de> wrote:

> Hi Brian,
> i like the the change in the code flow, but since the
> DeclarativeVersion (in your example) does not create the repository
> version anymore, i think it should be renamed.
> Maybe it's the SyncPipeline and we call perform on it instead of create.
>
I see what you're saying. What about the perspective that the
new_repository_version is the one DeclarativeVersion is acting upon? The
challenge w/ a name like SyncPipeline is that the Stages API aren't always
used for sync, for example the migration tool uses it for migrating
content. These are probably small semantic differences in the names, but
then again we're having a naming convo.


> Also i do not see the benefit of making the RelativePathFixer a context
> manager instead of a simple function to be called after the pipeline.
> It can even go wrong badly, if __exit__ is called after an exeption
> broke the pipeline.
>
I agree fully with your concerns and reasoning. I've shared examples w/ the
"context_manager" approach, but since it's never involved in error
handling, let's plan that the actual implementation all over would be using
the style like this gist had:
https://github.com/dralley/pulp_file/compare/typed-repositories...bmbouter:plugin-finalize-no-context-manager


> On Tue, 5 Nov 2019 14:46:18 -0500
> Brian Bouterse <bmbouter at redhat.com> wrote:
>
> > As a followup to the chat discussion from triage/open-floor today,
> > here is the POC on top of typed repositories. It's actually a very
> > small change, the *only* significant difference is that the stages
> > API no longer uses the RepositoryVersion context manager. Thus, the
> > plugin writer must finalize it, but they can do that using
> > core-provided facilities. The links below are diffs on top of
> > @dalley's unmerged PRs so the links are long:
> >
> >
> https://github.com/dralley/pulpcore/compare/typed-repositories...bmbouter:plugin-context-manager
> >
> https://github.com/dralley/pulp_file/compare/typed-repositories...bmbouter:plugin-context-manager
> >
> > I'm able to run the pulp-smash test with these changes so I think it's
> > working:
> > django-admin test
> > pulp_file.tests.functional.api.test_sync.BasicFileSyncTestCase.test_sync
> >
> > Note that the context manager is only syntactic sugar. The pulp_file
> > sync code could also just as easily be as shown below. This is
> > incomplete, but I think you'll get the idea.
> >
> >
> https://github.com/dralley/pulp_file/compare/typed-repositories...bmbouter:plugin-finalize-no-context-manager
> >
> > With this plugins can even do what they want in terms of style
> > (context manager or not). Also they can not use it at all and the
> > only extra responsibility would be to finalize the RepositoryVersion
> > with its context manager (core provided).
> >
> >
> > I'd like to ask for feedback on this design asap. Questions are
> > concerns ... please send 'em.
> >
> > An extensive description was given at open floor, but those logs
> > aren't up yet. The gist is that content modification/validation will
> > require user options, the plugin already knows that, so let's stop
> > having the core finalize the RepositoryVersion.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191105/25a2e72b/attachment.htm>