14:54:47 <dkliban> #startmeeting Pulp Triage 2019-10-01
14:54:47 <dkliban> #info dkliban has joined triage
14:54:47 <dkliban> !start
14:54:47 <pulpbot> Meeting started Tue Oct  1 14:54:47 2019 UTC.  The chair is dkliban. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:54:47 <pulpbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
14:54:47 <pulpbot> The meeting name has been set to 'pulp_triage_2019-10-01'
14:54:47 <pulpbot> dkliban: dkliban has joined triage
14:54:54 <dkliban> let's continue
14:55:17 <daviddavis> dalley: agreed. leave a comment on the issue.
14:55:25 <ttereshc> https://pulp.plan.io/issues/5353
14:55:54 <ttereshc> is it a concern?
14:56:21 <dkliban> yes it is
14:56:37 <bmbouter> agreed
14:57:00 <ttereshc> it seems like plugins don't use this type of relation... but it doesn't seem right in general
14:57:33 <bmbouter> I want to see the "redfined in pulpcore-plugin" part
14:57:45 <bmbouter> or I don't understand that part rather
14:58:02 <ttereshc> I'll send a link
14:58:25 <ttereshc> https://github.com/pulp/pulpcore-plugin/blob/2226678355ac09efab689be5ed0633522c2bed5e/pulpcore/plugin/models/remote.py#L10
14:58:50 <bmbouter> oic
14:58:59 <bmbouter> and it's because we redefine the meta?
14:59:04 <bmbouter> Meta
14:59:36 <ttereshc> good question, I'm not sure but it sounds reasonable
14:59:52 <ttereshc> otherwise it should be able to track back I guess
15:00:16 <dkliban> maybe
15:00:33 <bmbouter> do we remember why these aren't collapsed into a single model and not redeclared in pulpcore-plugin?
15:00:54 <dkliban> and not redeclared?
15:01:49 <bmbouter> redeclared meaning a new ancestor class in pulpcore-plugin that is inherited from pulpcore's defined one it even though the subclass is abstract
15:02:01 <dkliban> no idea
15:02:19 <dkliban> i thikn we should try to just move it all to pulpcore and only expose it in pulpcore-plugin
15:02:27 <bmbouter> recently we collapsed the Task "redeclaration" in this way
15:02:38 <bmbouter> dkliban: I agree
15:03:07 <bmbouter> I can't think of a reason why these methods on the object linked to by ttereshc would be unsafe to include in core itself
15:03:29 <ttereshc> I guess we need to rewrite this ticket to be more general, there is also ContentGuard and Content
15:04:02 <dkliban> bmbouter: the only downside i see is that you need to remember to bump the version of the plugin api when you change something in pulpcore
15:04:32 <bmbouter> dkliban: true but since it inherits from pulpcore we're already in that situation
15:04:53 <bmbouter> unless the changes were in those few methods maybe that's what you're sayin
15:05:14 <dkliban> yeah
15:05:32 <bmbouter> ttereshc: I'm not sure this problem is in many more places but maybe
15:05:48 <ttereshc> bmbouter, it's all in models directory
15:05:56 <bmbouter> daviddavis: the app_label solution we came up with that was that the "docs approach" or the "automated approach"?
15:06:14 <daviddavis> docs
15:06:41 <bmbouter> ttereshc: yes but do we redefine these in pulpcore-plugin for ContentGuard and Content for example?
15:06:42 <daviddavis> there was a code change that pulp will raise an exception if there's no default_related_name defined though
15:07:02 <ttereshc> bmbouter, yes https://github.com/pulp/pulpcore-plugin/blob/2226678355ac09efab689be5ed0633522c2bed5e/pulpcore/plugin/models/content.py
15:07:35 <ttereshc> bmbouter, I meant models directory in pulpcore-plugin, not in pulpcore
15:07:36 <bmbouter> ttereshc: yup you are right ty for that
15:08:02 <dkliban> so who's going to updat ethe ticket?
15:08:06 <ttereshc> I will
15:08:08 <bmbouter> ttereshc: I agree this issue should handle those 3 models together
15:08:13 <daviddavis> what's the solution?
15:08:24 <ttereshc> move everything to pulpcore
15:08:27 <dkliban> move all the code to pulpcore and just import it in pulpcore-plugin
15:08:39 <daviddavis> I see, thanks
15:08:42 <bmbouter> I think that would be best
15:08:56 <bmbouter> the other option is to add app_label = 'core' to the Meta for each
15:09:01 <bmbouter> just to name the other option
15:09:12 <bmbouter> but overall we want simpler things and this layer of inheritance seems out of place
15:09:18 <daviddavis> what about setting app_label to core for pulpcore plugin?
15:10:05 <ttereshc> daviddavis, how? if it's a separate app, it needs a unique app_label
15:10:32 <daviddavis> I see, yea
15:11:12 <daviddavis> +1 for removing inheritance from pulpcore plugin
15:11:12 <bmbouter> it's not a separate app though pulpcore-plugin isn't it's own django app right?
15:11:16 <bmbouter> +1 to that also
15:11:42 <ttereshc> bmbouter, yeah, it's not but to add app_label natively it will need to be I think
15:12:05 <bmbouter> mmm
15:12:12 <daviddavis> my only concern is that one day we'll want to extend a core model in the plugin API but I guess we can deal with that problem when we get to it
15:12:43 <bmbouter> yeah we can bring it back then at least then we'll have a known reason to separate (unlike today)
15:12:51 <daviddavis> yea
15:12:56 <ttereshc> there is a related issue to 5353 but inverted https://pulp.plan.io/issues/5355 Content defined in pulpcore-plugin is not used. I'll close this if we move everything to the pulpcore. Agreed?
15:13:09 <bmbouter> +1
15:13:15 <daviddavis> +1
15:13:42 <ttereshc> ok, so I'll update 5353 and close 5355
15:13:54 <ttereshc> I don't have anything else
15:14:53 <daviddavis> me neither
15:14:56 <daviddavis> anyone else?
15:14:57 <bmbouter> can we go back to these naming PRs
15:15:02 <daviddavis> sure
15:15:24 <daviddavis> !issue 4554
15:15:35 <bmbouter> these are the PRs that were announced on pulp-dev https://www.redhat.com/archives/pulp-dev/2019-September/msg00039.html
15:15:43 <dkliban> !issue 4554
15:15:44 <dkliban> #topic https://pulp.plan.io/issues/4554
15:15:45 <pulpbot> RM 4554 - ehelms@redhat.com - POST - Change naming of Pulp 3 services to differentiate them from Pulp 2 services
15:15:46 <pulpbot> https://pulp.plan.io/issues/4554
15:16:29 <dkliban> lmjachky: could you rebase https://github.com/pulp/ansible-pulp/pull/161
15:16:49 <ttereshc> dkliban, lmjachky has issues with irc at the moment, his messages are not getting sent
15:17:01 <ttereshc> but he can see your pings :)
15:17:13 <dkliban> oh ... he needs to identify with the NickServ
15:17:21 <ttereshc> yup, just told him
15:18:23 <ttereshc> dkliban, should he check all PRs for rebase or did you look at all them and only ansible is a concern?
15:18:49 <bmbouter> I just did a quick audit and maybe a quarter are unreviewed and half half requested changes
15:18:56 <bmbouter> mainly small things it seems
15:19:07 <dkliban> that's the main one that actually makes teh change
15:19:10 <dkliban> the rest are mostly docs
15:19:17 <ttereshc> ok thanks
15:19:20 <bmbouter> oh yeah
15:19:48 <ttereshc> dkliban, can we merge them after all plugins are released with rc6 compatibility?
15:19:56 <daviddavis> +1
15:20:01 <daviddavis> and not before
15:20:10 <dkliban> sure
15:20:15 <ttereshc> this is the downside of tagging only and not having a separate branch :/
15:20:25 <daviddavis> yup
15:20:38 <bmbouter> we could merge and the tag could be from a commit other than HEAD
15:20:49 <bmbouter> I'm not advocating for it, just identifying it
15:21:26 <bmbouter> +1 to merging after
15:21:35 <bmbouter> still though it needs some changes and some reviews
15:21:47 <daviddavis> yea
15:21:51 <dkliban> yeah
15:22:14 <bmbouter> overall though I think someone needs to be the advocate to move this forward, that's my main goal
15:22:41 <bmbouter> that could be lmjachky by pinging reviewers via github, or someone else, whatever you all are comfortable with
15:23:54 <dkliban> ype
15:24:26 <ttereshc> I can take care of that if it's not done today
15:25:18 <bmbouter> sweet, that would meet my needs
15:25:25 <daviddavis> yea, start tomorrow
15:25:30 <daviddavis> or whenever
15:25:32 <bmbouter> lol yeah what daviddavis said
15:25:37 <daviddavis> before the next release!
15:25:41 <dkliban> lol
15:26:03 <bmbouter> lmjachky: also great job on these PRs I really appreciate you making this change
15:26:14 <daviddavis> lmjachky++
15:26:14 <pulpbot> daviddavis: lmjachky's karma is now 5
15:26:19 <bmbouter> lmjachky++
15:26:19 <pulpbot> bmbouter: lmjachky's karma is now 6
15:26:21 <bmbouter> also for the pulp_file work
15:27:47 <bmbouter> ttereshc: also fyi I'm working on 5008 today
15:28:26 <ttereshc> bmbouter, great to hear that
15:28:56 <bmbouter> I don't have anything else
15:29:34 <ttereshc> Any thoughts on how to control how/what is removed from a repo https://pulp.plan.io/issues/5526?
15:30:00 <x9c4> bmbouter, can i have a review? https://github.com/pulp/ansible-pulp/pull/151
15:30:15 <ttereshc> #5526, it should be a default behaviour
15:30:38 <bmbouter> x9c4: yes but I think mikedep333 would do better than I on it. mikedep333 ^?
15:31:43 <bmbouter> ttereshc: modulemd is content right and it reference artifacts in the usual way, so I expected orphan cleanup would handle this
15:32:48 * bmbouter reviews the code
15:33:23 <bmbouter> ttereshc: I expected it would remove modulemd content as it becomes orphaned, and then eventually on the last one this second portion would run
15:33:41 <bmbouter> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/tasks/orphan.py#L31
15:34:36 <daviddavis> is open floor over?
15:34:38 <daviddavis> dkliban: ^
15:36:43 <dkliban> #endmeeting
15:36:43 <dkliban> !end