[Pulp-dev] [pulp-internal] Recommend #2950 be re-prioritized.

[Dennis Kliban]

I've posted a PR[0] that resolves this issue. I tested that I can upload
content, publish it, download it from a distribution. I also tested that I
can sync, publish, download from the distribution. The file path is also
correct when requesting an artifact via REST API.

[0] https://github.com/pulp/pulp/pull/3178

On Thu, Sep 28, 2017 at 1:27 PM, Jeff Ortel <jortel at redhat.com> wrote:

> On 09/28/2017 10:39 AM, Brian Bouterse wrote:
> > One of the things I heard was that we aren't sure why we have this
> custom storage backend> I was very surprised to hear that because it was
> developed and merged.
>
> Yes, implementing a custom storage back-end for no good reason would be
> surprising.  But, that's not what
> happened.
>
> The detailed reasons where very well understood at the time.  Basically,
> the behavior of the FileSystemStorage
> with regard to the way it calculated actual storage paths and how it
> stored files was incompatible with our
> needs.  I should have documented those exact details but didn't.  I was
> more interested in getting pulp3
> storage working.  Given the unexpected (and undocumented) behavior and
> code complexity in the
> FileSystemStorage, it seemed safer and easier to implement a few extra
> methods than to extend.
>
> > I want to make sure we understand custom code we've written before we go
> too much further. > I think that is why we put it on the sprint currently.
>
> If the purpose is to document requirements, do a gap analysis and
> re-design a solution, this task should not
> yet be groomed and aligned to a sprint.
>
> >
> > On Thu, Sep 28, 2017 at 11:06 AM, Jeff Ortel <jortel at redhat.com <mailto:
> jortel at redhat.com>> wrote:
> >
> >
> >
> >     On 09/28/2017 08:56 AM, Dennis Kliban wrote:
> >     > On Tue, Sep 26, 2017 at 11:14 AM, Jeff Ortel <jortel at redhat.com
> <mailto:jortel at redhat.com> <mailto:jortel at redhat.com <mailto:
> jortel at redhat.com>>> wrote:
> >     >
> >     >     Team,
> >     >
> >     >     I am fine with revisiting storage as some point but disagree
> that #2950 should be *high* priority (higher than
> >     >     most other tasks) and should not aligned with sprint 26.  As
> noted in redmine, Our FileStorage implementation
> >     >     conforms to the django storage interface, is simple and
> tested.  The django provided FileSystemStorage has
> >     >     concerning code quality and is completely undocumented.  To
> safely subclass it will require inspecting the
> >     >     code line-by-line to ensure predictable behavior when
> overriding any of it's methods.  As you all know,
> >     >     reliable storage is a critical part of Pulp.
> >     >
> >     >
> >     > We use the rest of Django without inspecting every line of code,
> so I don't see a reason to treat the
> >     > FileSystem storage backend any different. We are using Django so
> we can reduce the amount of code we are
> >     > maintaining ourselves. Completely reimplementing the storage
> backend goes against that goal. I plan to work on
> >     > this issue today.
> >
> >     The rest of django is documented.  The FileSystemStorage class is
> not.  Not even docstrings.  It has
> >     undocumented behaviors and the only way to understand them is to
> read the code.
> >
> >     I just have a hard time understanding why this is higher priority
> than these other sprint tasks like:
> >
> >     3024    content creation API does not validate the hostname portion
> of the URL.
> >     3021    Database writes are not all recorded in DB
> >     2994    Erratum not updated after upstream change
> >     2988    Exception when raising a user-Defined that has a custom
> __init__.
> >     2373    Planning on how to support global importer
> >
> >     And ... everything else left to do for the MVP.
> >
> >     >
> >     > -Dennis
> >     >
> >     >
> >     >     As I said, it's a fine idea to revisit this.  But, looking at
> the other tasks aligned to sprint 26 (and, all
> >     >     the work left to do for the MVP), this is not higher priority.
> >     >
> >     >     -jeff
> >     >
> >     >
> >     >     https://pulp.plan.io/issues/2950 <https://pulp.plan.io/issues/
> 2950>
> >     <https://pulp.plan.io/issues/2950 <https://pulp.plan.io/issues/2950
> >>
> >     >
> >     >
> >
> >
> >     _______________________________________________
> >     Pulp-dev mailing list
> >     Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
> >     https://www.redhat.com/mailman/listinfo/pulp-dev <
> https://www.redhat.com/mailman/listinfo/pulp-dev>
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20170928/15bacbd2/attachment.htm>