Bug#761149: debsources: allow redirects to package versions based on suite/codename

[Jason Pleau <jason@jpleau.ca>]

Hello Stefano,

Thanks for your feedback on this. I know how time consuming it can be to
review someone else's code when they're new to a project, so please know
I do appreciate it !

I have a couple of comments / questions to some of your suggestions :

On 13/11/14 04:37 PM, Stefano Zacchiroli wrote:
> 
>> diff --git a/debsources/migrate/007-to-008.sql b/debsources/migrate/007-to-008.sql
>> new file mode 100644
>> index 0000000..838346e
>> --- /dev/null
>> +++ b/debsources/migrate/007-to-008.sql
>> @@ -0,0 +1,14 @@
>> +ALTER TABLE suites_info
>> +  ADD COLUMN alias VARCHAR;
> 
> Regarding this, which is an important design decision here, I'm not
> particularly happy about having a maximum of 1 alias per suite. Why
> couldn't a suite have more?
> 
> Now, implement this properly in SQL would require a separate table to
> allow 1:N suite<->aliases mappings, which is probably a tad overkill for
> what we need here. So how about instead: 1/ renaming the new column to
> "aliases", and 2/ document that it is a comma-separated list of aliases,
> that all map to the current row in the suites_info table?
> 
> Of course your changes to views.py and models.py will need to be adapted
> accordingly.
> 

The idea of having multiple aliases per suite never crossed my mind. I'm
not sure it exists right now in Debian but that's a good idea.

The logical thing would be to create a separate table to allow the
association between a suite and its aliases. I think it would ease the
rest of the code since we'd let SQLAlchemy handle creating and saving
the 'aliases' array.

However if you're 100% certain that there will never be a need to link
specific data to a suite+alias association (I can't think of any),
storing a comma-separated string of aliases in the 'suites' table would
be fine for me.

>> +UPDATE suites_info
>> +  SET alias='unstable' WHERE name='sid';
>> +
>> +UPDATE suites_info
>> +  SET alias='testing' WHERE name='jessie';
>> +
>> +UPDATE suites_info
>> +  SET alias='stable' WHERE name='wheezy';
>> +
>> +UPDATE suites_info
>> +  SET alias='oldstable' WHERE name='squeeze';
> 
> This is another part of the design that bothers me a little. The above
> is OK for deploying quickly the change, but I certainly do not want to
> introduce another place in Debsources which will have to be manually
> maintained/updated at each Debian release.
> 
> So what I think we should do here is modifying the Debsources updated to
> retrieve aliases from Release files, and re-fill the suites_info table
> (including aliases information) at each update run. What do you think?
> 

Agreed about having the suites and their aliases auto-updated.

I remember we talked about it a bit on #debian-qa with pabs, there was
the possibility to use an API once it's finished:
https://api.ftp-master.debian.org/suites

If the API is not close to release, I can look into parsing Release
files, are you aware of a parsing tool in python? I was looking at
python-apt, but it seems to rely on a sources.list file, I don't think
that's not what we need.

>> @@ -194,15 +194,17 @@ class SuiteInfo(Base):
>>      version = Column(String, nullable=True)
>>      release_date = Column(Date, nullable=True)
>>      sticky = Column(Boolean, nullable=False)
>> +    alias = Column(String, nullable=True)
> 
> If you're OK with my proposed change, it'd be nice to have automatically
> split of the aliases list every time a SuiteInfo object is created, but
> I'm not sure what's the most appropriate way to do that in SQLAlchemy.
> If it's annoying to do, we can certainly live with keeping that as a
> comma-separated string and split it when needed.
> 

It would be annoying to have to split the comma-separated string every
time we want to make use of the aliases. I vote for splitting it into a
list (or tuple?) when a SuiteInfo object is instantiated. (That's if we
do it with a string, and not a 1:N mapping table)


> HTH,
> Cheers.
> 

Thanks

-- 
Jason Pleau