PR#5: Quick clean up - taskotron/resultsdb_conventions

		`@@ -29,6 +29,7 @@`
		`# pylint:disable=invalid-name`
		`logger = logging.getLogger(__name__)`

		`+`
		`class Result(object):`
		`"""Base class for a single result. The signature follows the API`
		`create_result class as closely as possible. outcome is the actual`

resultsdb_conventions/fedora.py

file modified

-1

		`@@ -53,7 +53,6 @@`
		`super(FedoraComposeResult, self).default_groups()`
		`# update the existing compose group with a URL`
		`try:`
		`- rel = fedfind.release.get_release(cid=self.cid)`
		`self.add_group('compose', self.cid, ref_url=self.ffrel.location)`
		`except (ValueError, fedfind.exceptions.FedfindError):`
		`logger.warning("fedfind found no release for compose ID %s, compose group will have no URL", self.cid)`

resultsdb_conventions/helpers.py

file modified

		`@@ -24,9 +24,11 @@`
		`import logging`
		`import uuid`

		`+`
		`# pylint:disable=invalid-name`
		`logger = logging.getLogger(__name__)`

		`+`
		`def uuid_namespace(name, namespace=None):`
		`"""Create a UUID using the provided name and namespace (or default`
		`DNS namespace), handling string type and encoding ickiness on both`

setup.py

file modified

+4 -3

		`@@ -17,11 +17,12 @@`
		`#`
		`# Author: Adam Williamson <awilliam@redhat.com>`

		`- import os`
		`import sys`
		`- from setuptools import setup, find_packages`
		`+`
		`+ from setuptools import setup`
		`from setuptools.command.test import test as TestCommand`

		`+`
		`class PyTest(TestCommand):`
		`user_options = [('pytest-args=', 'a', "Arguments to pass to py.test")]`

		`@@ -31,7 +32,7 @@`
		`self.test_suite = 'tests'`

		`def run_tests(self):`
		`- #import here, cause outside the eggs aren't loaded`
		`+ # import here, cause outside the eggs aren't loaded`
		`import pytest`
		`errno = pytest.main(self.pytest_args.split())`
		`sys.exit(errno)`

smilner commented 6 years ago

Removed unused imports
Removed unused variable
pep8 spacing
Add fedfind to install.requires

1 new commit added

install.requires: Add fedfind

6 years ago

Pull-Request has been merged by adamwill

6 years ago

adamwill commented 6 years ago

actually, the install.requires change is bogus - you added it right before I merged, unfortunately. fedfind is intentionally not listed there as it is an optional requirement; you only need it to use the Fedora classes. this project is quite carefully structured so you can use the bits of it you need and avoid the bits of it you don't and their dependencies; I figured it would impair adoption / usage for RHEL if there were hard dependencies on Fedora bits, and vice versa.

So you can import and use the classes from base.py and prodmd.py without needing anything Fedora-specific, for instance. Going forward the idea was there could be RH-specific classes with RH-specific dependencies, if desired, without Fedora usages needing to pull those in. And so on.

Note that setup.py actually expresses this: it defines an extras_require called fedora that requires fedfind.

I'll revert that part of the change.

Edited 6 years ago by adamwill

smilner commented 6 years ago

@adamwill appoligies. I didn't mean to get it in under the radar like that. I added it as when I installed resultsdb_conventions via pip I would get errors attempting to import the fedora compose class. Of course, I defer to you on what should or should not be in the requires files.

Again, sorry about that.

adamwill commented 6 years ago

I think pip install "resultsdb_conventions[fedora]" or something like that (I forget the precise syntax) should do the right thing. I'll try and make this clearer in the README.

Edited 6 years ago by adamwill