<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 5, 2019 at 3:53 PM Simon Baatz <<a href="mailto:gmbnomis@gmail.com" target="_blank">gmbnomis@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Brian,<br>
<br>
On Tue, Nov 05, 2019 at 02:46:18PM -0500, Brian Bouterse wrote:<br>
>    ...<br>
>    Note that the context manager is only syntactic sugar. The pulp_file<br>
>    sync code could also just as easily be as shown below. This is<br>
>    incomplete, but I think you'll get the idea.<br>
>    [3]<a href="https://github.com/dralley/pulp_file/compare/typed-repositories...bm" rel="noreferrer" target="_blank">https://github.com/dralley/pulp_file/compare/typed-repositories...bm</a><br>
>    bouter:plugin-finalize-no-context-manager<br>
>    With this plugins can even do what they want in terms of style (context<br>
>    manager or not). Also they can not use it at all and the only extra<br>
>    responsibility would be to finalize the RepositoryVersion with its<br>
>    context manager (core provided).<br>
>    I'd like to ask for feedback on this design asap. Questions are<br>
>    concerns ... please send 'em.<br>
<br>
This is an elegant solution for the sync case and, as you said,<br>
offers multiple options to implement validation/extra changes. <br>
However there are other places in core that create repo versions.  I<br>
think we have three overall: sync, add_and_remove task, and create()<br>
in SingleArtifactContentUploadSerializer.<br>
<br>
How will the plugin writer reuse validation code in the other places? <br>
Currently, it seems that he/she has to duplicate some of the existing<br>
core code to get the plugin's validation code into the right place.<br></blockquote><div><br></div><div>Thank you for bringing this up!</div><div><br></div><div>Here's one idea I had for the uploads use case:  <a href="https://gist.github.com/bmbouter/81be284e6296c541ef9c4a0efc9b304f">https://gist.github.com/bmbouter/81be284e6296c541ef9c4a0efc9b304f</a></div><div>Note the it's not the same pattern because core's code is still performing the finalization, but the plugin writer is able to pass in a fully configured context manager so in that sense it's not a "hook".</div><div><br></div><div>For the add_and_remove task, this change would be on top of typed repos. add_and_remove is called from only 1 place in @dralley's PR here: <a href="https://github.com/pulp/pulpcore/pull/354/files#diff-03cc41a4bcada47c767bb1615dc53cefR130">https://github.com/pulp/pulpcore/pull/354/files#diff-03cc41a4bcada47c767bb1615dc53cefR130</a> For that case we could decompose the 'modify' such that it could be used as-is if the plugin writer does not require the extra opportunity. If they do, then they could instantiate the plugin_context_manager in their viewset and pass it along to the task which could be modified like this:</div><div><br></div><div><a href="https://gist.github.com/bmbouter/0897648ce254faecbea730159f4e9cd1">https://gist.github.com/bmbouter/0897648ce254faecbea730159f4e9cd1</a></div><div><br></div><div>Another option is to move @modify from pulpcore to the plugins themselves, but then plugins have to opt-in instead of opt-out.</div><div><br></div><div>I expect instantiating the context manager in the viewset and passing the instance to the backend will work because RQ pickles it's job params: <a href="http://python-rq.org/docs/#on-the-design">http://python-rq.org/docs/#on-the-design</a></div><div><br></div><div>Note these gists aren't very "clean" but they should illustrate the idea. How about these approaches?<br></div><div><br></div><div><br></div><div><br></div><div><br></div></div></div>