On Wed, 2010-05-19 at 14:43 +0000, Данило Шеган wrote: > Hi Adi, > > Thanks a lot for the work on this. Overall, it's pretty good. > > У пон, 17. 05 2010. у 11:07 +0000, Adi Roiban пише: > > > == Tests == > > > > lib/lp/registry/stories/webservice/xx-distroseries.txt > > lib/lp/registry/stories/webservice/xx-project-registry.txt > > lib/lp/registry/stories/webservice/xx-source-package.txt > > lib/lp/translations/stories/webservice/xx-potemplate.txt > > It usually helps if you provide a bin/test command which runs them for > lazy reviewers to copy-paste them :) Sorry. I will add it with the next MP. ./bin/test -t xx-potemplate.txt -t xx-distroseries.txt \ -t xx-project-registry.txt -t xx-source-package.txt :) [snip] > > = Launchpad lint = > > > > Checking for conflicts. and issues in doctests and templates. > > Running jslint, xmllint, pyflakes, and pylint. > > Using normal rules. > > > > Linting changed files: > > lib/lp/translations/interfaces/webservice.py > > There are many more files with many more lint issues: you should try to > run it after you commit your final change, not before (if you've got > uncommitted changes, it lints just the uncommitted files). Please fix > all the issues. I have fixed some of the other warning. There are still some warnings for code like: @operation_parameters( pocket=Choice( title=_("Pocket"), required=True, vocabulary=DBEnumeratedType)) and it looks fine to me, but pylint complains about: lib/lp/registry/interfaces/sourcepackage.py 191: [C0322, ISourcePackage.getBranch] Operator not preceded by a space required=True, ^ vocabulary=DBEnumeratedType)) > > == Pyflakes notices == > > > > lib/lp/translations/interfaces/webservice.py > > 8: 'IHasTranslationImports' imported but unused > > 8: 'ITranslationImportQueue' imported but unused > > 8: 'ITranslationImportQueueEntry' imported but unused > > 13: 'IPOTemplate' imported but unused > > 15: 'IPOFile' imported but unused > > You should add the following to webservice.py if that's the intention: > > __all__ = [ > 'IHasTranslationImports', > 'IPOFile', > 'IPOTemplate', > 'ITranslationImportQueue', > 'ITranslationImportQueueEntry', > ] > > But, why is this necessary? Only interfaces listed in webservices.py will be exported by lazr.restful. Adding __all__ will still generate Pyflakes notices as they are still unused in that file. > > warning: Not importing directory '/usr/share/pyshared/lazr': missing > __init__.py > > warning: Not importing directory '/usr/share/pyshared/lazr': missing > __init__.py > > warning: Not importing directory '/usr/share/pyshared/lazr': missing > __init__.py > > This might be a bug in lazr that needs fixing. Can you please check and > file appropriate bug if it is? Is is not a problem in lazr. I have the same problem with: '/usr/share/pyshared/google': missing __init__.py Those file were installed as dependencies for ubuntuone-client-gnome by python-protobuf and python-lazr-uri, python-lazr-restfulclient and are maintained by main Ubuntu developers... not in the LP PPA. > > === modified file 'lib/canonical/launchpad/security.py' > > --- lib/canonical/launchpad/security.py 2010-05-17 12:43:53 > +0000 > > +++ lib/canonical/launchpad/security.py 2010-05-19 12:09:43 > +0000 > > @@ -61,8 +61,7 @@ from lp.hardwaredb.interfaces.hwdb impor > > from lp.services.worlddata.interfaces.language import ILanguage, > ILanguageSet > > from lp.translations.interfaces.languagepack import ILanguagePack > > from canonical.launchpad.interfaces.launchpad import ( > > - IBazaarApplication, IHasBug, IHasDrivers, ILaunchpadCelebrities, > > - IPersonRoles) > > + IHasBug, IHasDrivers, ILaunchpadCelebrities, IPersonRoles) > > from lp.registry.interfaces.role import IHasOwner > > from lp.registry.interfaces.location import IPersonLocation > > from lp.registry.interfaces.mailinglist import IMailingListSet > > @@ -1159,6 +1158,11 @@ class AdminDistributionTranslations(Auth > > self.obj).checkAuthenticated(user)) > > > > > > +class ViewPOTemplates(AnonymousAuthorization): > > + """Anyone can view an IPOTemplate.""" > > + usedfor = IPOTemplate > > + > > + > > class AdminPOTemplateDetails(OnlyRosettaExpertsAndAdmins): > > """Controls administration of an `IPOTemplate`. > > > > @@ -1211,6 +1215,11 @@ class AddPOTemplate(OnlyRosettaExpertsAn > > usedfor = IProductSeries > > > > > > +class ViewPOFile(AnonymousAuthorization): > > + """Anyone can view an IPOFile.""" > > + usedfor = IPOFile > > + > > + > > class EditPOFileDetails(EditByOwnersOrAdmins): > > usedfor = IPOFile > > Why were these needed? Isn't this the default? Especially since they > don't seem to be registered anywhere. AnonymousAuthorization is required for allowing anonymous access to the API, otherwise the POTemplate and POFile API will only be available to logged in users. Should POTemplate and POFile API be available only to authenticated users? > === modified file > 'lib/lp/registry/stories/webservice/xx-distroseries.txt' > === modified file > 'lib/lp/registry/stories/webservice/xx-project-registry.txt' > === modified file > 'lib/lp/registry/stories/webservice/xx-source-package.txt' > > It would be nice if we could unify these. And, I actually believe these > additions should be put into lib/lp/translations/stories/webservice > test instead: they are testing the IHasTranslationTemplates, not the > above objects. I moved them into lib/lp/translations/stories/webservice/xx-potemplate.txt > > === modified file 'lib/lp/translations/interfaces/potemplate.py' > ... > > source_file = Object( > > title=_('Source file for this translation template'), > > readonly=True, schema=ILibraryFileAlias) > > I don't remember why we decided not to export this one? It could be > useful for things like xpipo conversion (with XPI-based translations, > it should contain a reference to imported en-US.xpi). > > Though, we should probably just export it as the public URL, so not a > big deal (let's not block this branch on this). I will leave it for another branch, but for me, it would make sense if this attribute would be used for all translation templates, not only XPI. > > === modified file 'lib/lp/translations/interfaces/webservice.py' > > --- lib/lp/translations/interfaces/webservice.py 2009-07-17 > 02:25:09 > +0000 > > +++ lib/lp/translations/interfaces/webservice.py 2010-05-19 > 12:09:43 > +0000 > > @@ -1,9 +1,16 @@ > > # Copyright 2009 Canonical Ltd. This software is licensed under the > > # GNU Affero General Public License version 3 (see the file LICENSE). > > > > +# pylint: disable-msg=W0611 > > + > > """All the interfaces that are exposed through the webservice.""" > > > > from lp.translations.interfaces.translationimportqueue import ( > > IHasTranslationImports, > > ITranslationImportQueue, > > ITranslationImportQueueEntry) > > + > > +from lp.translations.interfaces.potemplate import ( > > + IPOTemplate) > > +from lp.translations.interfaces.pofile import ( > > + IPOFile) > > See comment above on the lint output. I added # pylint: disable-msg=W0611, but I don't know what else can be done to avoid the pyflakes warnings. Here is the latest diff: === modified file 'lib/lp/registry/interfaces/distroseries.py' --- lib/lp/registry/interfaces/distroseries.py 2010-05-17 09:35:18 +0000 +++ lib/lp/registry/interfaces/distroseries.py 2010-05-19 17:04:11 +0000 @@ -594,7 +594,8 @@ :param dsc: string, original content of the dsc file :param copyright: string, the original debian/copyright content :param changelog: LFA ID of the debian/changelog file in librarian - :param changelog_entry: string, changelog extracted from the changesfile + :param changelog_entry: string, changelog extracted from the + changesfile :param architecturehintlist: string, DSC architectures :param builddepends: string, DSC build dependencies :param builddependsindep: string, DSC architecture independent build === modified file 'lib/lp/registry/model/distroseries.py' --- lib/lp/registry/model/distroseries.py 2010-05-17 09:35:18 +0000 +++ lib/lp/registry/model/distroseries.py 2010-05-19 17:04:11 +0000 @@ -1299,15 +1299,13 @@ find_spec = ( DistroSeriesPackageCache, BinaryPackageName, - SQL('rank(fti, ftq(%s)) AS rank' % sqlvalues(text)) - ) + SQL('rank(fti, ftq(%s)) AS rank' % sqlvalues(text))) origin = [ DistroSeriesPackageCache, Join( BinaryPackageName, DistroSeriesPackageCache.binarypackagename == - BinaryPackageName.id - ) + BinaryPackageName.id), ] # Note: When attempting to convert the query below into straight @@ -1321,7 +1319,7 @@ DistroSeriesPackageCache.name ILIKE '%%' || %s || '%%') """ % (quote(self), quote(self.distribution.all_distro_archive_ids), - quote(text), quote_like(text)) + quote(text), quote_like(text)), ).config(distinct=True) # Create a function that will decorate the results, converting === modified file 'lib/lp/registry/stories/webservice/xx-distroseries.txt' --- lib/lp/registry/stories/webservice/xx-distroseries.txt 2010-05-18 15:27:05 +0000 +++ lib/lp/registry/stories/webservice/xx-distroseries.txt 2010-05-19 17:04:11 +0000 @@ -99,27 +99,3 @@ ... Location: http://.../ubuntu/+milestone/alpha1 ... - - -Getting all potemplates for a distroseries ------------------------------------------- - -All templates associated to a distribution series are available at the -'all_translation_templates' collection link. - - >>> from zope.component import getUtility - >>> from canonical.launchpad.interfaces.launchpad import ( - ... ILaunchpadCelebrities) - >>> from lp.translations.interfaces.potemplate import IPOTemplateSet - >>> login('