15:30:46 <fao89> #startmeeting Pulp Triage 2019-11-05
15:30:46 <fao89> #info fao89 has joined triage
15:30:46 <fao89> !start
15:30:46 <pulpbot> Meeting started Tue Nov  5 15:30:46 2019 UTC.  The chair is fao89. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:30:46 <pulpbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
15:30:46 <pulpbot> The meeting name has been set to 'pulp_triage_2019-11-05'
15:30:46 <pulpbot> fao89: fao89 has joined triage
15:31:05 <dawalker> #info dawalker has joined triage
15:31:05 <dawalker> !here
15:31:05 <pulpbot> dawalker: dawalker has joined triage
15:31:10 <daviddavis> #info daviddavis has joined triage
15:31:10 <daviddavis> !here
15:31:11 <pulpbot> daviddavis: daviddavis has joined triage
15:31:19 <fao89> !next
15:31:20 <pulpbot> fao89: 3 issues left to triage: 5665, 5660, 5656
15:31:21 <fao89> #topic https://pulp.plan.io/issues/5665
15:31:21 <pulpbot> RM 5665 - mdellweg - NEW - Publications do not have a natural key and are not searchable by repository version.
15:31:22 <pulpbot> https://pulp.plan.io/issues/5665
15:31:22 <ggainey> #info ggainey has joined triage
15:31:22 <ggainey> !here
15:31:23 <pulpbot> ggainey: ggainey has joined triage
15:31:26 <ttereshc> #info ttereshc has joined triage
15:31:26 <ttereshc> !here
15:31:26 <pulpbot> ttereshc: ttereshc has joined triage
15:31:27 <ppicka> #info ppicka has joined triage
15:31:27 <ppicka> !here
15:31:27 <pulpbot> ppicka: ppicka has joined triage
15:32:08 <bmbouter> #info bmbouter has joined triage
15:32:08 <bmbouter> !here
15:32:08 <pulpbot> bmbouter: bmbouter has joined triage
15:32:23 <ttereshc> sounds like a story
15:32:31 <daviddavis> +1
15:32:32 <ggainey> yeah, was thinking same
15:32:35 <ggainey> +1
15:32:38 <fao89> #idea Proposed for #5665: convert to a story
15:32:38 <fao89> !propose other convert to a story
15:32:38 <pulpbot> fao89: Proposed for #5665: convert to a story
15:32:44 <dawalker> +1
15:32:48 <fao89> #agreed convert to a story
15:32:48 <fao89> !accept
15:32:49 <pulpbot> fao89: Current proposal accepted: convert to a story
15:32:50 <fao89> #topic https://pulp.plan.io/issues/5660
15:32:50 <pulpbot> fao89: 2 issues left to triage: 5660, 5656
15:32:51 <pulpbot> RM 5660 - lmjachky - NEW - A broken link in the pulp documentation
15:32:52 <pulpbot> https://pulp.plan.io/issues/5660
15:33:26 <daviddavis> accept + add to sprint + easyfix tag
15:33:33 <ggainey> +1
15:33:39 <ppicka> +1
15:33:42 <ttereshc> +1
15:33:47 <fao89> #idea Proposed for #5660: accept, add to sprint
15:33:47 <fao89> !propose other accept, add to sprint
15:33:47 <pulpbot> fao89: Proposed for #5660: accept, add to sprint
15:33:50 <fao89> #agreed accept, add to sprint
15:33:50 <fao89> !accept
15:33:50 <pulpbot> fao89: Current proposal accepted: accept, add to sprint
15:33:51 <pulpbot> fao89: 1 issues left to triage: 5656
15:33:51 <fao89> #topic https://pulp.plan.io/issues/5656
15:33:52 <pulpbot> RM 5656 - daviddavis - NEW - Creating a FileRemote and RpmRemote with the same name raises an error
15:33:53 <pulpbot> https://pulp.plan.io/issues/5656
15:34:15 <bmbouter> accept, not add to sprint?
15:34:22 <daviddavis> +1
15:34:25 <ttereshc> you and your friday, daviddavis
15:34:29 <daviddavis> :)
15:34:44 <fao89> #idea Proposed for #5656: Leave the issue as-is, accepting its current state.
15:34:44 <fao89> !propose accept
15:34:44 <pulpbot> fao89: Proposed for #5656: Leave the issue as-is, accepting its current state.
15:34:58 <ttereshc> +1
15:35:17 <fao89> #agreed Leave the issue as-is, accepting its current state.
15:35:17 <fao89> !accept
15:35:17 <pulpbot> fao89: Current proposal accepted: Leave the issue as-is, accepting its current state.
15:35:18 <pulpbot> fao89: No issues to triage.
15:35:24 <fao89> Open floor!
15:36:03 <bmbouter> dalley and I are working on resolving the issue gmbnomis identified (ty gmbnomis)
15:36:32 <daviddavis> issue 5627
15:36:44 <fao89> !issue 5627
15:36:45 <fao89> #topic https://pulp.plan.io/issues/5627
15:36:45 <pulpbot> RM 5627 - daviddavis - NEW - Remove plugin managed repos
15:36:47 <pulpbot> https://pulp.plan.io/issues/5627
15:36:56 <daviddavis> thoughts on adding this to the sprint?
15:37:12 <bmbouter> I just don't think we can b/c it's blocked still
15:37:18 <bmbouter> s/just//
15:37:24 <daviddavis> yea, I set the blocker
15:38:14 <daviddavis> I was just thinking someone could pick this up immediately after we merge the typed repo PR
15:39:09 <ttereshc> I was hoping someone will pick up 3541 immediately after typed repos PR :)
15:39:23 <bmbouter> so 3541 is the next topic to discuss...
15:39:24 <daviddavis> I think it's being worked on
15:39:34 <bmbouter> it is
15:39:52 <bmbouter> I want to write the recap of what dalley and I learned yesterday and what we think the solution is w.r.t 3541
15:40:12 <ttereshc> great, I was about to ask
15:40:21 <ttereshc> I saw you had a discussion yesterday
15:40:35 <bmbouter> I plan to write to the list but I'd like to share here
15:41:16 <bmbouter> the issue is that we can't have a pattern where the call order is user -> plugin code -> core code -> plugin code for additional modification of content
15:42:00 <bmbouter> because the user's options/params are lost in the plugin -> core -> plugin
15:42:15 <bmbouter> which is an irony because the plugin code (the first one) already has all the options
15:43:46 <ttereshc> what's the proposed solution? subclass repoversion?
15:43:53 <bmbouter> so here's the overall thing I've learned from this:   we very much liked the idea of a "single context manager that can be used in all places that can provide plugin specific validation"
15:44:19 <bmbouter> so what we could do is have plugins do just that
15:44:55 <bmbouter> and stop using the RepositoryVersion context manager. current that context manager does 2 thing: deletion of invalid/failed RepoVersions and setting complete=True
15:45:39 <bmbouter> https://github.com/pulp/pulpcore/blob/a9bf2e54dec7f9a576e39fea191d586eb84769f3/pulpcore/app/models/repository.py#L629-L647
15:46:28 <daviddavis> couldn't we provide those two things to plugins somehow?
15:46:41 <bmbouter> that's the RepositoryVersion context manager I'm speaking of (that we currently have)
15:47:13 <bmbouter> we can provide some helping things but the crux of the design problem we have is that core is trying to do it
15:47:37 <daviddavis> right, I was imagining some methods on repo version
15:47:41 <bmbouter> sure
15:47:58 <bmbouter> ok so with all ^ in mind here's the proposal
15:48:00 <daviddavis> or a method I guess
15:48:08 <gmbnomis> Couldn't the plugin context manager just user the current one?
15:48:09 <bmbouter> 1) remove the RepoVersion context manager
15:48:14 <gmbnomis> s/user/use/
15:48:22 <bmbouter> I don't think so
15:48:42 <bmbouter> because in the core code itself the RepoVersion is "finalized" shown to the user and becomes immutable
15:48:48 <bmbouter> which is the crux of it actually
15:49:38 <gmbnomis> But why can't the validation happen before leaving the "inner" contect manager?
15:49:55 <bmbouter> it could still provide the failure cleanup
15:49:55 <bmbouter> it's the complete=True happening in core that we need to stop
15:49:55 <bmbouter> so let me adjust step (1) ...  1) remove the complete=True finalization from RepoVersion context manager
15:50:38 <bmbouter> well if the inner one occurs in core and it has to call plugin code to perform the validation then we loose the params
15:50:52 <bmbouter> so the callback pattern should be avoided
15:51:30 <bmbouter> so the plugin should provide a context manager that performs modification/validation
15:51:35 <bmbouter> let's assume that for a second
15:51:40 <bmbouter> as a thought experiment
15:51:45 <gmbnomis> oh, I think we mean different things
15:52:15 <bmbouter> having two context managers makes the convo complicated
15:52:21 <bmbouter> can we imagine the RepoVersion doesn't have one anymore
15:52:29 <gmbnomis> sure
15:52:56 <bmbouter> so the plugin provides a context manager and it wraps the core code anywhere core code is used to meaningfully modify a repoverion's content
15:53:02 <bmbouter> for example in sync for pulp_file
15:53:34 <bmbouter> so the plugin's context manager would wrap this line for example
15:53:35 <bmbouter> https://github.com/pulp/pulp_file/blob/master/pulp_file/app/tasks/synchronizing.py#L45
15:54:28 <bmbouter> and if core is no longer setting complete=True then core can do everything it's supposed to and then let the plugin writer handle everything they want to in their context manager's __exit__
15:55:00 <bmbouter> and this solves the issue because the context manager can receive all of it's config from user data a few lines above
15:55:26 * bmbouter listens
15:55:50 <daviddavis> that makes sense to me. I wonder if it would be simpler (or possible) though to let a plugin define a repo version save stage in their code?
15:56:19 <bmbouter> the issue is that then "it can't beused in all places same same"
15:56:34 <bmbouter> that would work for sync but things that don't use stages now need to solve it again
15:56:50 <daviddavis> it could if the plugin writer creates some methods for it or whatnot
15:57:09 <daviddavis> my concern is that we're essentially doing the hooks proposal that we talked about before
15:57:24 <bmbouter> we're not though because the call pattern is different
15:57:25 <daviddavis> and ceding control of repo version saving to core
15:57:37 <bmbouter> the hook was a callback pattern
15:57:48 <bmbouter> this is not a callback it turns the core code into a subroutine
15:57:53 <daviddavis> it feels very similar though
15:58:06 <bmbouter> well we liked that proposal except for the callback problem
15:58:14 <bmbouter> that was the only issue we had with it to my memory
15:58:39 <daviddavis> yea, I see the benefit of having plugins define a context manager
15:59:08 <daviddavis> I'm just trying to outline some downsides I see
15:59:19 <bmbouter> sure and I want to listen for them
16:00:16 <daviddavis> I guess core would also control the modify endpoint and use these context managers to allow plugins to modify/validate content?
16:00:58 <bmbouter> it could but the generic handling of inputs into that context manager could be complicated but maybe doable
16:01:22 <gmbnomis> A different question: How does the plugin manager get the RepoVersion created within https://github.com/pulp/pulp_file/blob/master/pulp_file/app/tasks/synchronizing.py#L45?
16:01:42 <bmbouter> gmbnomis: well that's the other implication ... core shouldn't create that anymore
16:04:19 <bmbouter> daviddavis: the issue w/ one endpoint will come in the openAPI schema ... which options does it support would be impossible to know
16:04:41 <bmbouter> this is part of the motivation to move all repo verison endpoints to plugin code also
16:04:50 <bmbouter> i.e. typed repositories
16:05:00 <daviddavis> +1 from me
16:05:31 <daviddavis> what are the next steps?
16:05:59 <bmbouter> we were going to prototype on top of the typed repo PR so we could throw it away if it doesn't work out
16:06:02 <bmbouter> dalley and I today
16:06:14 <bmbouter> ttereshc: and this is effectively 3541
16:06:19 <daviddavis> great, I think seeing a prototype will help
16:06:31 <bmbouter> gmbnomis: wdyt about all this?
16:07:49 <gmbnomis> honestly, I have a hard time to imagine how this will look.
16:08:20 <bmbouter> I hear that it's a huge amount of info and in a very compressed timeline
16:08:32 <gmbnomis> But I am not attached to the "hooky" handler solution at all. So, I think let's try it out.
16:08:32 <bmbouter> so this prototype along w/ a redux explanation of ^ would be send to pulp-dev asap
16:09:51 <gmbnomis> +1 to that approach
16:10:32 <bmbouter> definitly want input/feedback when that happens. I'm hoping it goes out in a few hours
16:10:47 <bmbouter> feel free to chat more about it but that's the update for open floor
16:10:58 <ttereshc> bmbouter, thanks, it was helpful. Thanks for recap, +1 to try it out. For my development, (just not to be blocked) I added a hook into the current context manager but I agree with all the problems it brings and I'm not suggesting that
16:11:36 <bmbouter> yes your PR's code would need to be rearranged a bit but mostly in tact I think
16:11:48 <bmbouter> so I think you can continue as-is and port onto this change at the end perhaps
16:12:50 <bmbouter> the other thing that is hapening is CONTENT_ORIGIN we finally got it fully passing on travis!
16:13:23 <bmbouter> do any plugins use CONTENT_HOST today other than pulp_ansible and pulp_docker?
16:13:29 <bmbouter> I'll make Prs for any that do
16:13:47 <bmbouter> actually we'll have to open PRs against all projects so I can answer this myself as I go
16:14:06 <bmbouter> because your Travis environments will need to set the vars (but we know how to do that easly now so it's just a few line diff)
16:14:27 <ttereshc> yeah, apart from travis, rpm and migration plugin don;t use it
16:14:37 <bmbouter> cool
16:15:32 <fao89> #endmeeting
16:15:32 <fao89> !end