IArchive.syncSources returned value is unhelpful

Bug #372946 reported by Celso Providelo
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Medium
Celso Providelo

Bug Description

The current implementation of IArchive.syncSources() returns 'a list of source package names copied' when it succeeds. This information doesn't contain anything new to call sites, when it's returned it happens to be exactly the same list given to the method as 'source_names' argument.

It would be useful to call sites to know exactly which versions were copied, since (intentionally) it can't be specified on calls. But then we have a problem because of the set-nature of the method, it would have to return a list of (name, version) tuples, and it gets even worse by the fact that the copied set can also include binaries ... It would have to a dictionary ... We all know that dealing with arbitrary data-structures is asking for trouble, they are hard to document which easily leads to misuse and unhappy users.

With that said, the most reasonable thing we can do is to focus on the transactional behavior of syncSources. More important than the ability to copy source A, B and C in the same request, syncSource allow users to copy them altogether in the same transaction, i.e, A will only be copied the destination archive if B and C can be copied as well, otherwise the method will fail. That's a very useful aspect of this operation that seems to be hidden by all the underlying issues related to the lack of useful resulting code.

In this scenario, my vote is to modify the code to not return anything, which is cleaner and expected from a transactional operation. Call sites will have to lookup the destination archives again for finding out fresh references to the copies, when and if they need to. Additionally we should improve the documentation for highlighting the situations this method applies to (which are not clear atm).

Tags: api lp-soyuz
Revision history for this message
William Grant (wgrant) wrote :

Agreed - the current return value isn't useful, and anything significantly more useful is very difficult and/or confusing.

Also +1 on the docstring changes, as I wasn't aware of this transactional behaviour.

Revision history for this message
Celso Providelo (cprov) wrote :

r8759 (devel)

Changed in soyuz:
milestone: pending → 2.2.7
status: Triaged → Fix Committed
Celso Providelo (cprov)
Changed in soyuz:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.