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