launchpad-dev team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

On Thursday 23 July 2009 14:11:44 Celso Providelo wrote:
> Right, in the same way we implicitly check packagesets when querying
> permission for a given package name, we could also check against the
> source *current* component if we knew the context distroseries, no
> need to callsites to call the canUpload() again.

Ah, very good point, I hadn't thought of doing it right there in canUpload(), 
because my vision is clouded by the mess in NascentUpload and its 
peculiarities regarding different error messages (as you mentioned below).

> Based on the discussion we had on #lp-dev yesterday we need to
> consolidate all this logic in a single point, something like:
>
> can_upload(user, archives, source_name, distroseries, pocket) -> Yes or
> None

Are you suggesting to move this away from IArchive.canUpload()?  I don't think 
that's a good idea if so. Also, is archives (plural) a typo?

>   * check upload right to the pocket (archive, distroseries, pocket)

I think we need to get more clever about pocket here.  We should be able to 
default it to RELEASE or UPDATES at least, depending on the distroseries 
state.  Jono would still need to pass it of course, since he has an object 
referring to a suite.

>   * check upload rights on the source_name/packageset (archive,
> source_name)
> * figure out the component where the source is currently
> published (archive, distroseries)

What about pocket?  I don't think I've seen the component change between 
pockets but it's possible, right?  Or we can just take all info from the most 
recent publication.

>   * check upload rights on the component (archive, component)

Perfect!  But, we only need to do this if distroseries < karmic soon, unless 
Colin needs us to use both components and packagesets still.

> This function can immediately replace NascentUpload.verify_acl() and
> UploadPolicy.checkUpload() in archiveuploader/ if it respects some
> specialties (checks for users with no permissions at all ate treated
> differently).

We need to think about the ancestry checks as well and avoid doing them twice.

> I think refactoring the existing code in archiveuploader/ domain is
> the easier and path to get what we need for package-branches without
> introducing any, possibly unneeded, extra abstraction to the system.
>
> We can think about benefiting of the check in the zope permission domain
> later.

+1