14:30:21 <fao89> #startmeeting Pulp Triage 2020-04-24 14:30:21 <fao89> !start 14:30:21 <fao89> #info fao89 has joined triage 14:30:21 <pulpbot> Meeting started Fri Apr 24 14:30:21 2020 UTC. The chair is fao89. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:30:21 <pulpbot> Useful Commands: #action #agreed #help #info #idea #link #topic. 14:30:21 <pulpbot> The meeting name has been set to 'pulp_triage_2020-04-24' 14:30:21 <pulpbot> fao89: fao89 has joined triage 14:30:26 <daviddavis> #info daviddavis has joined triage 14:30:26 <daviddavis> !here 14:30:26 <pulpbot> daviddavis: daviddavis has joined triage 14:30:26 <x9c4> #info x9c4 has joined triage 14:30:26 <x9c4> !here 14:30:27 <pulpbot> x9c4: x9c4 has joined triage 14:30:27 <ppicka> #info ppicka has joined triage 14:30:27 <ppicka> !here 14:30:28 <dkliban> #info dkliban has joined triage 14:30:28 <dkliban> !here 14:30:29 <pulpbot> ppicka: ppicka has joined triage 14:30:30 <pulpbot> dkliban: dkliban has joined triage 14:30:30 <ggainey> #info ggainey has joined triage 14:30:30 <ggainey> !here 14:30:31 <pulpbot> ggainey: ggainey has joined triage 14:30:31 <fao89> !next 14:30:32 <pulpbot> fao89: 7 issues left to triage: 6571, 6568, 6564, 6537, 6534, 6496, 6463 14:30:32 <fao89> #topic https://pulp.plan.io/issues/6571 14:30:32 <mikedep333> #info mikedep333 has joined triage 14:30:32 <mikedep333> !here 14:30:34 <pulpbot> RM 6571 - daviddavis - ASSIGNED - Check for bad changelog entry extensions 14:30:35 <pulpbot> https://pulp.plan.io/issues/6571 14:30:36 <pulpbot> mikedep333: mikedep333 has joined triage 14:30:49 <dkliban> #idea Proposed for #6571: accept and add to sprint 14:30:49 <dkliban> !propose other accept and add to sprint 14:30:49 <pulpbot> dkliban: Proposed for #6571: accept and add to sprint 14:30:51 <fao89> !propose other accept and add to sprint 14:30:53 <daviddavis> +1 14:31:04 <fao89> #agreed accept and add to sprint 14:31:04 <fao89> !accept 14:31:04 <pulpbot> fao89: Current proposal accepted: accept and add to sprint 14:31:05 <fao89> #topic https://pulp.plan.io/issues/6568 14:31:05 <pulpbot> fao89: 6 issues left to triage: 6568, 6564, 6537, 6534, 6496, 6463 14:31:06 <pulpbot> RM 6568 - mped - NEW - Pulp 3 Sync of CentOS 8 Base OS repo, also ends up including AppStream 14:31:07 <pulpbot> https://pulp.plan.io/issues/6568 14:31:13 <ttereshc> #info ttereshc has joined triage 14:31:13 <ttereshc> !here 14:31:13 <pulpbot> ttereshc: ttereshc has joined triage 14:31:23 <dkliban> sounds like a feature 14:31:42 <daviddavis> move to rpm 14:31:43 <dkliban> i believe it's because of kickstarts 14:32:00 <ttereshc> yes and it's a correct behaviour 14:32:09 <dkliban> #idea Proposed for #6568: move to RPM 14:32:09 <dkliban> !propose other move to RPM 14:32:09 <pulpbot> dkliban: Proposed for #6568: move to RPM 14:32:11 <ttereshc> +1 to move to rpm 14:32:15 <ttereshc> and I can comment 14:32:16 <fao89> #agreed move to RPM 14:32:16 <fao89> !accept 14:32:16 <pulpbot> fao89: Current proposal accepted: move to RPM 14:32:17 <fao89> #topic https://pulp.plan.io/issues/6564 14:32:18 <pulpbot> fao89: 5 issues left to triage: 6564, 6537, 6534, 6496, 6463 14:32:19 <pulpbot> RM 6564 - daviddavis - NEW - Export filename for pulp exports has dupe slashes 14:32:20 <pulpbot> https://pulp.plan.io/issues/6564 14:32:24 <daviddavis> accept 14:32:28 <ggainey> +1 14:32:29 <dalley> #info dalley has joined triage 14:32:29 <dalley> !here 14:32:29 <pulpbot> dalley: dalley has joined triage 14:32:30 <x9c4> +1 14:32:33 <dkliban> +1 14:32:37 <fao89> #idea Proposed for #6564: Leave the issue as-is, accepting its current state. 14:32:37 <fao89> !propose accept 14:32:38 <pulpbot> fao89: Proposed for #6564: Leave the issue as-is, accepting its current state. 14:32:40 <ppicka> +1 14:32:43 <fao89> #agreed Leave the issue as-is, accepting its current state. 14:32:43 <fao89> !accept 14:32:43 <pulpbot> fao89: Current proposal accepted: Leave the issue as-is, accepting its current state. 14:32:44 <fao89> #topic https://pulp.plan.io/issues/6537 14:32:44 <pulpbot> fao89: 4 issues left to triage: 6537, 6534, 6496, 6463 14:32:45 <pulpbot> RM 6537 - daviddavis - NEW - Write a guide for debugging tasks 14:32:46 <pulpbot> https://pulp.plan.io/issues/6537 14:32:53 <daviddavis> arg task 14:32:58 <dalley> we have some docs for doing this in pulp 2 I think 14:33:01 <dkliban> #idea Proposed for #6537: convert to task 14:33:01 <dkliban> !propose other convert to task 14:33:01 <pulpbot> dkliban: Proposed for #6537: convert to task 14:33:03 <fao89> #idea Proposed for #6537: convert to a task 14:33:04 <fao89> !propose other convert to a task 14:33:04 <pulpbot> fao89: Proposed for #6537: convert to a task 14:33:14 <dkliban> +1 14:33:17 <dalley> that can partially be copied 14:33:22 <fao89> #agreed convert to a task 14:33:22 <fao89> !accept 14:33:22 <pulpbot> fao89: Current proposal accepted: convert to a task 14:33:23 <fao89> #topic https://pulp.plan.io/issues/6534 14:33:23 <pulpbot> fao89: 3 issues left to triage: 6534, 6496, 6463 14:33:24 <daviddavis> dalley: cool can you add a link to the issue? 14:33:25 <pulpbot> RM 6534 - ttereshc - NEW - Having same content in one batch can cause issues in _post_save of ContentSaver 14:33:25 <dalley> not py-spy though, that would be useful to document 14:33:26 <pulpbot> https://pulp.plan.io/issues/6534 14:33:30 <bmbouter> #info bmbouter has joined triage 14:33:30 <bmbouter> !here 14:33:30 <pulpbot> bmbouter: bmbouter has joined triage 14:33:42 <fao89> #idea Proposed for #6534: Leave the issue as-is, accepting its current state. 14:33:42 <fao89> !propose accept 14:33:42 <pulpbot> fao89: Proposed for #6534: Leave the issue as-is, accepting its current state. 14:33:46 <daviddavis> +1 14:33:51 <ttereshc> +1 14:33:58 <x9c4> +1 14:33:59 <fao89> #agreed Leave the issue as-is, accepting its current state. 14:33:59 <fao89> !accept 14:33:59 <pulpbot> fao89: Current proposal accepted: Leave the issue as-is, accepting its current state. 14:34:00 <ppicka> +1 14:34:00 <fao89> #topic https://pulp.plan.io/issues/6496 14:34:01 <pulpbot> fao89: 2 issues left to triage: 6496, 6463 14:34:02 <pulpbot> RM 6496 - dalley - NEW - Created Resources created and removed later are displayed as null in the task 14:34:03 <pulpbot> https://pulp.plan.io/issues/6496 14:34:27 <dkliban> should we add this to the sprint? 14:34:34 <dkliban> i just don't want to forget about it 14:34:36 <dalley> I think so 14:34:40 <ttereshc> yeah 14:34:47 <fao89> #idea Proposed for #6496: accept and add to sprint 14:34:47 <fao89> !propose other accept and add to sprint 14:34:47 <pulpbot> fao89: Proposed for #6496: accept and add to sprint 14:34:53 <x9c4> +1 14:34:55 <fao89> #agreed accept and add to sprint 14:34:55 <fao89> !accept 14:34:55 <pulpbot> fao89: Current proposal accepted: accept and add to sprint 14:34:56 <fao89> #topic https://pulp.plan.io/issues/6463 14:34:56 <pulpbot> fao89: 1 issues left to triage: 6463 14:34:57 <pulpbot> RM 6463 - binlinf0 - NEW - pulp 3.2.1 duplicate key error when sync 14:34:58 <pulpbot> https://pulp.plan.io/issues/6463 14:35:14 <ttereshc> long time no see 14:35:15 <bmbouter> +1 14:35:18 <bmbouter> +1 14:35:29 <bmbouter> whoops I was scrolled back 14:35:40 * bmbouter retracts +1s 14:35:51 <dkliban> i think we can close this one ... he is not able to reproduce 14:35:58 <bmbouter> I agree 14:36:08 <dkliban> i can close it and leave a coment 14:36:18 <bmbouter> we can't accept w/o reproduer so close and skip are only options and I'm +1 to close NOTABUG 14:36:23 <fao89> #idea Proposed for #6463: close and leave a comment 14:36:23 <fao89> !propose other close and leave a comment 14:36:23 <pulpbot> fao89: Proposed for #6463: close and leave a comment 14:36:27 <ttereshc> +1 14:36:29 <fao89> #agreed close and leave a comment 14:36:29 <fao89> !accept 14:36:29 <pulpbot> fao89: Current proposal accepted: close and leave a comment 14:36:29 <x9c4> +1 14:36:30 <dkliban> +1 14:36:31 <pulpbot> fao89: No issues to triage. 14:36:39 <fao89> Open floor! 14:36:49 <dkliban> https://github.com/pulp/pulpcore/pull/600 14:37:04 <dkliban> i wanted to start by saying that this PR is addressing multiple issues 14:37:34 <dkliban> 1. how to handle write_only fields in our openapi schema 14:37:58 <dkliban> 2. converting username and password fields on a remote to be SecretCharFields 14:38:20 <dkliban> so i propose that we open a separate issue for 2 14:38:27 <dkliban> and discuss 1 now 14:38:48 <ttereshc> +1 to address different issues in different PRs 14:38:49 <daviddavis> I think 2 can be handled by https://pulp.plan.io/issues/6421 14:39:27 <dkliban> daviddavis: maybe 14:39:36 <daviddavis> it specifically says that "write_only fields can be converted to SecretCharFields" 14:40:10 <dkliban> ok ... i am fine with that ... bmbouter, could you please provide some comments on 6421 ? 14:40:19 <bmbouter> one of my open floor topics is that we should not use secretCharFields at all 14:40:33 <fao89> and I want to bring attention to x9c4 comment: https://github.com/pulp/pulpcore/pull/600#issuecomment-617612095 how write_only could be a problem in the future 14:40:35 <daviddavis> ok, maybe that is different 14:40:48 <dkliban> fao89: yes, that's topic 1 from above 14:41:02 <dkliban> so let's dig into that 14:41:02 <daviddavis> let's file an issue for bmbouter's point and/or discuss after this 14:41:05 <daviddavis> +1 14:41:13 <bmbouter> my take on why we shouldn't use SecretCharField is that we should wait for RBAC to only show plaintext data to users who have rightful access to it 14:41:23 <bmbouter> we can discuss later/after is fine w/ me 14:41:31 <dkliban> ok .. thanks bmbouter 14:41:49 <fao89> yesterday ttereshc posted an issue with SecretCharField 14:42:21 <daviddavis> so the issue with write_only fields is that we can't use them 14:42:27 <dkliban> so with regard to write_only fields and our openapi schema, i think x9c4 brought up a very good point that if we auto generate serializers for serializers that have write_only fields, we will constatnly be breaking our clients 14:42:41 <dkliban> so i agree with daviddavis 14:42:56 <daviddavis> does anyone not agree? 14:43:14 <x9c4> Or we do the split in to sChema serializers for all models. 14:43:27 <daviddavis> yes good point 14:43:29 * daviddavis cries 14:44:17 <dkliban> if we don't use write_only fields, we will have to split up serializers 14:44:22 <bmbouter> I think we can't use write_only fields 14:44:42 <daviddavis> yea I agree 14:44:46 <bmbouter> but not for *all* serializers right? just the ones that have fields that couldn't be read again? 14:45:07 <dkliban> that's right ... i think this only affects Content serializers 14:45:13 <daviddavis> that's my understanding. teh dev or plugin writer has to manually split up serializers 14:45:18 <fao89> here are all the write_only we current use: https://pulp.plan.io/issues/6346#note-10 14:45:54 <dkliban> we use write_only incorrectly sometimes though ... if the serializer is only used for POST operations, we dont' need to mark those fields as write_only 14:46:07 <daviddavis> yup 14:46:27 <dkliban> so i really think this is only a problem for Content which can be created using a 'file' but it doesnt' have a 'file' attribute on GET 14:46:48 <dkliban> so the other option is to include 'file' for GET and give it a value of 'not set for GET" 14:47:17 <dkliban> i would actually prefer something like that 14:47:27 <bmbouter> dkliban: that is also the scope of the problem AIUI (assuming secret fields no longer need to be secret due to RBAC) 14:48:22 <bmbouter> dkliban: so you're saying it would just be null or something on GET? 14:48:40 <dkliban> bmbouter: yeah ... or a path to the artifact if we want to reveal that. 14:49:11 <bmbouter> w/ things like NoArtifactSingleContentUploader it wouldn't have an artifact but yes 14:49:39 <bmbouter> how does that work w/ PATCH? 14:50:03 <x9c4> You cannot patch content. 14:50:04 <dkliban> that method is not supoprted for content ... but i should verify that 14:50:28 <dkliban> bmbouter: you have to delete and recreate content 14:50:43 <bmbouter> can someone take an AI to summarize the current plan shortly on pulp-dev? 14:50:53 <dkliban> yes, i'll do it 14:50:54 <x9c4> But with passwords on remotes? 14:50:55 <daviddavis> why is this solution better than having read vs write serializers for content? 14:51:01 <fao89> I found the issue related to SecretCharFields https://pulp.plan.io/issues/6565 14:51:12 <bmbouter> I also don't understand that @daviddavis 14:51:22 <dkliban> daviddavis: because plugin writers have to do less 14:51:26 <bmbouter> I think I'm in favor of explicit serializers for the cases we need them for 14:51:43 <bmbouter> the tradeoff is tho, less work for plugin writers more confusion for users 14:51:49 <daviddavis> oh ok 14:52:02 <bmbouter> and the size of these relative groups to me suggests we should optizmize onless confusion for users 14:52:18 <dkliban> a good point 14:52:33 <bmbouter> can the summery include both options 14:52:38 <dkliban> oh for sure 14:52:39 <bmbouter> and also triage + open floor needs to be longer 14:52:47 <daviddavis> yea, I have a couple more questions about this path 14:52:51 <bmbouter> yup 14:52:52 <dkliban> which path? 14:52:59 <daviddavis> of not using write_onyl fields 14:53:03 <dkliban> oh ok 14:53:03 <fao89> and we have to make some validations, otherwise write_only fields will bite us again in the future 14:53:11 <daviddavis> this ^ 14:53:23 <daviddavis> can we raise an exception or something? 14:53:38 <dkliban> yes, when we are loading everything at startup pulp can fail to start 14:53:55 <daviddavis> cool, that would be good 14:54:06 <daviddavis> my other question was just about getting this documented 14:54:10 <dkliban> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/apps.py#L49 14:54:34 <dkliban> we need to add a method that imports serializers and does validation 14:55:31 <dkliban> yeah ... we need to add plugin writer docs that say that write_only is a no no 14:55:37 <x9c4> If we have different Serializers for read and write, does the schema reflect that? 14:55:43 <dkliban> yes 14:56:07 <dkliban> and our cli code will be able to easily know when to use which 14:56:21 <bmbouter> yeah I think we need that the other option is seeming like a no go to me for these reasons 14:56:22 <x9c4> Nice. 14:56:52 <dkliban> bmbouter: can you rephrase that more specifically 14:57:36 <dkliban> with more specificity 14:57:48 <bmbouter> +1 to splitting serializers because of the benefits of the schema becoming explicit 14:58:07 <bmbouter> -1 to using one serializer with fields that can only be used in some options but are present in the schema for areas they can't be used 14:58:40 <dkliban> bmbouter: yes, that would make it more confusing to know when a field should be rendered for the user or when not 14:59:07 <dkliban> i'll still outline both options in my note with all the pros and cons 15:00:03 <dkliban> my initial note pulp-list said this change would come with 3.4 15:00:16 <dkliban> do we have reason to believe that is still possible? 15:00:43 <bmbouter> it's reasonable enough not to cancel it 15:00:47 <dkliban> cool 15:00:49 <bmbouter> unfortunately I have to go to another meeting 15:01:07 <bmbouter> so I propose we make triage + open floor a 1 hour meeting for T and Friday 15:01:16 <dkliban> sounds good 15:01:18 <bmbouter> we do not have enough time together 15:01:26 <daviddavis> yea 15:01:28 <dkliban> i love you guys! 15:01:34 <daviddavis> ha 15:01:34 <fao89> hahaha 15:01:52 <daviddavis> can we open an issue for the secretcharfields or start a discussion on pulp-dev? 15:01:54 <dkliban> cool ... i think we can adjourn for today 15:02:03 <bmbouter> fao89: as the lead can you make the schedule change? 15:02:16 <fao89> sure 15:02:53 <dkliban> bmbouter: please open an issue about secretcharfields 15:05:16 <fao89> I cannot change triage schedule, dkliban is the owner 15:06:56 <fao89> #endmeeting