Re: Breaking up the importer's huge icommon.py file
> Max Bowsher <_...@maxb.eu> writes: > SubprocessMonitor and ThreadDriver look fairly generic - the only > reasons I wanted to merge them in was to spare me having to come > up with a good name for a separate file to put them, and it > doesn't seem likely that they'll need to be re-used outside > mass_import.py My main reason to keep them *outside* of mass_import.py was to ensure I wasn't accidentally mixing the features provided there with the specific needs of mass_import (from memory, mainly db access and logging). I won't mind an ithreads.py module, you don't even have to (but that would be nice of course ;) to split the related tests (which I think are already not separated in test_mass_import ;) -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
On 24/05/11 17:57, James Westby wrote: > On Sat, 21 May 2011 18:52:20 +0100, Max Bowsher wrote: >> A huge amount of the UDD importer's interesting code is in one file, >> icommon.py. >> >> I'd like to submit a series of changes to break it up such that only the >> most common bits of code remain there. > > Good idea. > >> Things I'm thinking of: >> >> 1) Move code in icommon.py that is uniquely invoked by a single >> executable component of the importer out to that component: > > I think I would prefer it if they moved to their own modules... > >> e.g. >> class ImportList, class PackageToImport --> import_package.py >> >> class SubprocessMonitor, class ThreadDriver --> mass_import.py > > Especially things like this. > > I don't have a strong preference though. ImportList and PackageToImport really are very much core parts of the logic of how import_package.py works. SubprocessMonitor and ThreadDriver look fairly generic - the only reasons I wanted to merge them in was to spare me having to come up with a good name for a separate file to put them, and it doesn't seem likely that they'll need to be re-used outside mass_import.py Max. signature.asc Description: OpenPGP digital signature -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
On Sat, 21 May 2011 18:52:20 +0100, Max Bowsher wrote: > A huge amount of the UDD importer's interesting code is in one file, > icommon.py. > > I'd like to submit a series of changes to break it up such that only the > most common bits of code remain there. Good idea. > Things I'm thinking of: > > 1) Move code in icommon.py that is uniquely invoked by a single > executable component of the importer out to that component: I think I would prefer it if they moved to their own modules... > e.g. > class ImportList, class PackageToImport --> import_package.py > > class SubprocessMonitor, class ThreadDriver --> mass_import.py Especially things like this. I don't have a strong preference though. Thanks, James -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
Max Bowsher <_...@maxb.eu> wrote: >> Max Bowsher wrote: >>> "package_importer" is a bit long. "udd" works for me, especially as >the >>> project lives in lp:udd. > >On 23/05/11 12:19, Scott Kitterman wrote: >> The term udd is, unfortunately, overloaded. It's also 'Ultimate >> Debian Database'. I suggest something that avoids that. Maybe >> ubuntudd or uddev? > >The conflict is unfortunate. The importer's Launchpad project ID is >udd, >though. Given these Python modules are not going to be packaged or even >used other than by files in the same tree, renaming them at a later >date >is trivial - therefore, I am in favour of matching the Python module >name to the Launchpad project ID - and if we change both of them at the >same time at some later date, that's fine. > To the extent Launchpad is the world, I think that's fine. Scott K-- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
> Max Bowsher wrote: >> "package_importer" is a bit long. "udd" works for me, especially as the >> project lives in lp:udd. On 23/05/11 12:19, Scott Kitterman wrote: > The term udd is, unfortunately, overloaded. It's also 'Ultimate > Debian Database'. I suggest something that avoids that. Maybe > ubuntudd or uddev? The conflict is unfortunate. The importer's Launchpad project ID is udd, though. Given these Python modules are not going to be packaged or even used other than by files in the same tree, renaming them at a later date is trivial - therefore, I am in favour of matching the Python module name to the Launchpad project ID - and if we change both of them at the same time at some later date, that's fine. Max. signature.asc Description: OpenPGP digital signature -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
Max Bowsher <_...@maxb.eu> wrote: >On 23/05/11 07:42, Andrew Bennetts wrote: >> Max Bowsher wrote: >>> A huge amount of the UDD importer's interesting code is in one file, >>> icommon.py. >>> >>> I'd like to submit a series of changes to break it up such that only >the >>> most common bits of code remain there. >> >> This all sounds pretty reasonable to me. >> >> However I don't much care for the “ifoo” module naming convention. I >> assume the “i” stands for “importer” but it always makes me think >> “interface” first. >> >> If we want a namespace for these modules (and I think we do; >namespaces >> are a good idea[1]) then let's do that the standard Python way: with >a >> package. Because I'm uncreative with names I suggest calling it >> “package_importer”, or perhaps “udd”. So rather than “idatabase” I >> propose “package_importer.database”. What do you think? >> >> This would have the advantage of keeping the directory of scripts >that >> are interesting for a person to run mostly separate from the >libraries >> used to implement them. At the moment they're jumbled together. >> >> -Andrew. >> >> [1] Actually, not just good, honking great: python -c 'import this' > >"package_importer" is a bit long. "udd" works for me, especially as the >project lives in lp:udd. The term udd is, unfortunately, overloaded. It's also 'Ultimate Debian Database'. I suggest something that avoids that. Maybe ubuntudd or uddev? Scott K -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
> Max Bowsher <_...@maxb.eu> writes: > "package_importer" is a bit long. "udd" works for me, especially as the > project lives in lp:udd. +1 > Once we have a namespace, I'm inclined to move the tests there too - > that would sidestep naughty packages which install top-level "tests" > modules on the system path. +1 Vincent -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
> Max Bowsher writes: > A huge amount of the UDD importer's interesting code is in one > file, icommon.py. > I'd like to submit a series of changes to break it up such that > only the most common bits of code remain there. +1 > Assuming the idea is liked, I think the best way to proceed would > be for me to do a single one of these moves, merge-propose it, > wait for approval, land it, and repeat the cycle until done. Sounds like a good plan. I've been willing to do that too, the main risk is to miss a dependency for a given script and catch it only when it breaks. I've tried to identify all scripts and record where they were used (see importer.crontab and etc-init.d-mass-import for some) but I'm not sure I got them all. I know this sounds a bit like FUD at this point so don't get too scared either ;) I won't mind additional smoke blackbox tests to cover that (but won't block a patch because of that either). > This should minimize clashing with other branches trying to land, > and potentially awkward merges. /me nods Vincent -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
On 23/05/11 07:42, Andrew Bennetts wrote: > Max Bowsher wrote: >> A huge amount of the UDD importer's interesting code is in one file, >> icommon.py. >> >> I'd like to submit a series of changes to break it up such that only the >> most common bits of code remain there. > > This all sounds pretty reasonable to me. > > However I don't much care for the “ifoo” module naming convention. I > assume the “i” stands for “importer” but it always makes me think > “interface” first. > > If we want a namespace for these modules (and I think we do; namespaces > are a good idea[1]) then let's do that the standard Python way: with a > package. Because I'm uncreative with names I suggest calling it > “package_importer”, or perhaps “udd”. So rather than “idatabase” I > propose “package_importer.database”. What do you think? > > This would have the advantage of keeping the directory of scripts that > are interesting for a person to run mostly separate from the libraries > used to implement them. At the moment they're jumbled together. > > -Andrew. > > [1] Actually, not just good, honking great: python -c 'import this' "package_importer" is a bit long. "udd" works for me, especially as the project lives in lp:udd. Once we have a namespace, I'm inclined to move the tests there too - that would sidestep naughty packages which install top-level "tests" modules on the system path. Max. signature.asc Description: OpenPGP digital signature -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Re: Breaking up the importer's huge icommon.py file
Max Bowsher wrote: > A huge amount of the UDD importer's interesting code is in one file, > icommon.py. > > I'd like to submit a series of changes to break it up such that only the > most common bits of code remain there. This all sounds pretty reasonable to me. However I don't much care for the “ifoo” module naming convention. I assume the “i” stands for “importer” but it always makes me think “interface” first. If we want a namespace for these modules (and I think we do; namespaces are a good idea[1]) then let's do that the standard Python way: with a package. Because I'm uncreative with names I suggest calling it “package_importer”, or perhaps “udd”. So rather than “idatabase” I propose “package_importer.database”. What do you think? This would have the advantage of keeping the directory of scripts that are interesting for a person to run mostly separate from the libraries used to implement them. At the moment they're jumbled together. -Andrew. [1] Actually, not just good, honking great: python -c 'import this' -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
Breaking up the importer's huge icommon.py file
A huge amount of the UDD importer's interesting code is in one file, icommon.py. I'd like to submit a series of changes to break it up such that only the most common bits of code remain there. Things I'm thinking of: 1) Move code in icommon.py that is uniquely invoked by a single executable component of the importer out to that component: e.g. class ImportList, class PackageToImport --> import_package.py class SubprocessMonitor, class ThreadDriver --> mass_import.py load_explanations --> categorise_failures.py 2) Where a category of code with a single theme exists, move it from icommon.py to a new file - e.g.: move all code dealing with the Launchpad API to a new ilpapi.py. move all code dealing with sqlite dbs to a new idatabase.py. Assuming the idea is liked, I think the best way to proceed would be for me to do a single one of these moves, merge-propose it, wait for approval, land it, and repeat the cycle until done. This should minimize clashing with other branches trying to land, and potentially awkward merges. Max. signature.asc Description: OpenPGP digital signature -- ubuntu-distributed-devel mailing list ubuntu-distributed-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-distributed-devel
