Pylint fixing base_could_client,acloud_main,avd and common_operations.
- fix pylint errors
- Add some pylint disable comments to avoid pylint error temporarily
Bug: None
Test: run pylint
Change-Id: Ibf2ca67631d540cb6dc8d2fa463225cb52860a1e
diff --git a/public/acloud_main.py b/public/acloud_main.py
index cdf37eb..823bafb 100644
--- a/public/acloud_main.py
+++ b/public/acloud_main.py
@@ -13,7 +13,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
r"""Cloud Android Driver.
This CLI manages google compute engine project for android devices.
@@ -65,7 +64,6 @@
import argparse
import getpass
import logging
-import os
import sys
# TODO: Find a better way to handling this import logic.
@@ -90,6 +88,7 @@
CMD_SSHKEY = "project_sshkey"
+# pylint: disable=too-many-statements
def _ParseArgs(args):
"""Parse args.
@@ -99,8 +98,9 @@
Returns:
Parsed args.
"""
- usage = ",".join([CMD_CREATE, CMD_CREATE_CUTTLEFISH, CMD_DELETE,
- CMD_CLEANUP, CMD_SSHKEY])
+ usage = ",".join([
+ CMD_CREATE, CMD_CREATE_CUTTLEFISH, CMD_DELETE, CMD_CLEANUP, CMD_SSHKEY
+ ])
parser = argparse.ArgumentParser(
description=__doc__,
formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -124,10 +124,11 @@
dest="branch",
help="Android branch, e.g. mnc-dev or git_mnc-dev")
# TODO: Support HEAD (the latest build)
- create_parser.add_argument("--build_id",
- type=str,
- dest="build_id",
- help="Android build id, e.g. 2145099, P2804227")
+ create_parser.add_argument(
+ "--build_id",
+ type=str,
+ dest="build_id",
+ help="Android build id, e.g. 2145099, P2804227")
create_parser.add_argument(
"--spec",
type=str,
@@ -135,24 +136,26 @@
required=False,
help="The name of a pre-configured device spec that we are "
"going to use. Choose from: %s" % ", ".join(constants.SPEC_NAMES))
- create_parser.add_argument("--num",
- type=int,
- dest="num",
- required=False,
- default=1,
- help="Number of instances to create.")
+ create_parser.add_argument(
+ "--num",
+ type=int,
+ dest="num",
+ required=False,
+ default=1,
+ help="Number of instances to create.")
create_parser.add_argument(
"--gce_image",
type=str,
dest="gce_image",
required=False,
help="Name of an existing compute engine image to reuse.")
- create_parser.add_argument("--local_disk_image",
- type=str,
- dest="local_disk_image",
- required=False,
- help="Path to a local disk image to use, "
- "e.g /tmp/avd-system.tar.gz")
+ create_parser.add_argument(
+ "--local_disk_image",
+ type=str,
+ dest="local_disk_image",
+ required=False,
+ help="Path to a local disk image to use, "
+ "e.g /tmp/avd-system.tar.gz")
create_parser.add_argument(
"--no_cleanup",
dest="no_cleanup",
@@ -179,7 +182,8 @@
action="store_true",
dest="autoconnect",
required=False,
- help="For each instance created, we will automatically creates both 2 ssh"
+ help=
+ "For each instance created, we will automatically creates both 2 ssh"
" tunnels forwarding both adb & vnc. Then add the device to adb.")
subparser_list.append(create_parser)
@@ -238,7 +242,8 @@
action="store_true",
dest="autoconnect",
required=False,
- help="For each instance created, we will automatically creates both 2 ssh"
+ help=
+ "For each instance created, we will automatically creates both 2 ssh"
" tunnels forwarding both adb & vnc. Then add the device to adb.")
subparser_list.append(create_cf_parser)
@@ -307,7 +312,8 @@
action="store_true",
dest="autoconnect",
required=False,
- help="For each instance created, we will automatically creates both 2 ssh"
+ help=
+ "For each instance created, we will automatically creates both 2 ssh"
" tunnels forwarding both adb & vnc. Then add the device to adb.")
subparser_list.append(create_gf_parser)
@@ -355,12 +361,12 @@
dest="ssh_rsa_path",
required=True,
help="Absolute path to the file that contains the public rsa key "
- "that will be added as project-wide ssh key.")
+ "that will be added as project-wide ssh key.")
subparser_list.append(sshkey_parser)
# Add common arguments.
- for p in subparser_list:
- acloud_common.AddCommonArguments(p)
+ for parser in subparser_list:
+ acloud_common.AddCommonArguments(parser)
return parser.parse_args(args)
@@ -402,8 +408,8 @@
raise errors.CommandArgError(
"%s is not valid. Choose from: %s" %
(parsed_args.spec, ", ".join(constants.SPEC_NAMES)))
- if not ((parsed_args.build_id and parsed_args.build_target) or
- parsed_args.gce_image or parsed_args.local_disk_image):
+ if not ((parsed_args.build_id and parsed_args.build_target)
+ or parsed_args.gce_image or parsed_args.local_disk_image):
raise errors.CommandArgError(
"At least one of the following should be specified: "
"--build_id and --build_target, or --gce_image, or "
@@ -414,21 +420,22 @@
if parsed_args.which in [CMD_CREATE_CUTTLEFISH, CMD_CREATE_GOLDFISH]:
if not parsed_args.build_id or not parsed_args.build_target:
- raise errors.CommandArgError("Must specify --build_id and --build_target")
+ raise errors.CommandArgError(
+ "Must specify --build_id and --build_target")
if parsed_args.which == CMD_CREATE_GOLDFISH:
if not parsed_args.emulator_build_id:
- raise errors.CommandArgError("Must specify --emulator_build_id")
+ raise errors.CommandArgError("Must specify --emulator_build_id")
if parsed_args.which in [
- CMD_CREATE, CMD_CREATE_CUTTLEFISH, CMD_CREATE_GOLDFISH
+ CMD_CREATE, CMD_CREATE_CUTTLEFISH, CMD_CREATE_GOLDFISH
]:
- if (parsed_args.serial_log_file and
- not parsed_args.serial_log_file.endswith(".tar.gz")):
+ if (parsed_args.serial_log_file
+ and not parsed_args.serial_log_file.endswith(".tar.gz")):
raise errors.CommandArgError(
"--serial_log_file must ends with .tar.gz")
- if (parsed_args.logcat_file and
- not parsed_args.logcat_file.endswith(".tar.gz")):
+ if (parsed_args.logcat_file
+ and not parsed_args.logcat_file.endswith(".tar.gz")):
raise errors.CommandArgError(
"--logcat_file must ends with .tar.gz")
@@ -514,8 +521,8 @@
logcat_file=args.logcat_file,
autoconnect=args.autoconnect)
elif args.which == CMD_DELETE:
- report = device_driver.DeleteAndroidVirtualDevices(cfg,
- args.instance_names)
+ report = device_driver.DeleteAndroidVirtualDevices(
+ cfg, args.instance_names)
elif args.which == CMD_CLEANUP:
report = device_driver.Cleanup(cfg, args.expiration_mins)
elif args.which == CMD_SSHKEY:
diff --git a/public/actions/common_operations.py b/public/actions/common_operations.py
index 74ee50c..5a6d1d2 100644
--- a/public/actions/common_operations.py
+++ b/public/actions/common_operations.py
@@ -13,7 +13,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
"""Common operations between managing GCE and Cuttlefish devices.
This module provides the common operations between managing GCE (device_driver)
@@ -22,7 +21,6 @@
"""
import logging
-import os
from acloud.public import avd
from acloud.public import errors
@@ -33,127 +31,133 @@
def CreateSshKeyPairIfNecessary(cfg):
- """Create ssh key pair if necessary.
+ """Create ssh key pair if necessary.
- Args:
- cfg: An Acloudconfig instance.
+ Args:
+ cfg: An Acloudconfig instance.
- Raises:
- error.DriverError: If it falls into an unexpected condition.
- """
- if not cfg.ssh_public_key_path:
- logger.warning("ssh_public_key_path is not specified in acloud config. "
- "Project-wide public key will "
- "be used when creating AVD instances. "
- "Please ensure you have the correct private half of "
- "a project-wide public key if you want to ssh into the "
- "instances after creation.")
- elif cfg.ssh_public_key_path and not cfg.ssh_private_key_path:
- logger.warning("Only ssh_public_key_path is specified in acloud config, "
- "but ssh_private_key_path is missing. "
- "Please ensure you have the correct private half "
- "if you want to ssh into the instances after creation.")
- elif cfg.ssh_public_key_path and cfg.ssh_private_key_path:
- utils.CreateSshKeyPairIfNotExist(cfg.ssh_private_key_path,
- cfg.ssh_public_key_path)
- else:
- # Should never reach here.
- raise errors.DriverError("Unexpected error in CreateSshKeyPairIfNecessary")
+ Raises:
+ error.DriverError: If it falls into an unexpected condition.
+ """
+ if not cfg.ssh_public_key_path:
+ logger.warning(
+ "ssh_public_key_path is not specified in acloud config. "
+ "Project-wide public key will "
+ "be used when creating AVD instances. "
+ "Please ensure you have the correct private half of "
+ "a project-wide public key if you want to ssh into the "
+ "instances after creation.")
+ elif cfg.ssh_public_key_path and not cfg.ssh_private_key_path:
+ logger.warning(
+ "Only ssh_public_key_path is specified in acloud config, "
+ "but ssh_private_key_path is missing. "
+ "Please ensure you have the correct private half "
+ "if you want to ssh into the instances after creation.")
+ elif cfg.ssh_public_key_path and cfg.ssh_private_key_path:
+ utils.CreateSshKeyPairIfNotExist(cfg.ssh_private_key_path,
+ cfg.ssh_public_key_path)
+ else:
+ # Should never reach here.
+ raise errors.DriverError(
+ "Unexpected error in CreateSshKeyPairIfNecessary")
class DevicePool(object):
- """A class that manages a pool of virtual devices.
+ """A class that manages a pool of virtual devices.
- Attributes:
- devices: A list of devices in the pool.
- """
-
- def __init__(self, device_factory, devices=None):
- """Constructs a new DevicePool.
-
- Args:
- device_factory: A device factory capable of producing a goldfish or
- cuttlefish device. The device factory must expose an attribute with
- the credentials that can be used to retrieve information from the
- constructed device.
- devices: List of devices managed by this pool.
- """
- self._devices = devices or []
- self._device_factory = device_factory
- self._compute_client = device_factory.GetComputeClient()
-
- def CreateDevices(self, num):
- """Creates |num| devices for given build_target and build_id.
-
- Args:
- num: Number of devices to create.
+ Attributes:
+ devices: A list of devices in the pool.
"""
- # Create host instances for cuttlefish/goldfish device.
- # Currently one instance supports only 1 device.
- for _ in range(num):
- instance = self._device_factory.CreateInstance()
- ip = self._compute_client.GetInstanceIP(instance)
- self.devices.append(
- avd.AndroidVirtualDevice(ip=ip, instance_name=instance))
+ def __init__(self, device_factory, devices=None):
+ """Constructs a new DevicePool.
- def WaitForBoot(self):
- """Waits for all devices to boot up.
+ Args:
+ device_factory: A device factory capable of producing a goldfish or
+ cuttlefish device. The device factory must expose an attribute with
+ the credentials that can be used to retrieve information from the
+ constructed device.
+ devices: List of devices managed by this pool.
+ """
+ self._devices = devices or []
+ self._device_factory = device_factory
+ self._compute_client = device_factory.GetComputeClient()
- Returns:
- A dictionary that contains all the failures.
- The key is the name of the instance that fails to boot,
- and the value is an errors.DeviceBootError object.
- """
- failures = {}
- for device in self._devices:
- try:
- self._compute_client.WaitForBoot(device.instance_name)
- except errors.DeviceBootError as e:
- failures[device.instance_name] = e
- return failures
+ def CreateDevices(self, num):
+ """Creates |num| devices for given build_target and build_id.
- @property
- def devices(self):
- """Returns a list of devices in the pool.
+ Args:
+ num: Number of devices to create.
+ """
- Returns:
- A list of devices in the pool.
- """
- return self._devices
+ # Create host instances for cuttlefish/goldfish device.
+ # Currently one instance supports only 1 device.
+ for _ in range(num):
+ instance = self._device_factory.CreateInstance()
+ ip = self._compute_client.GetInstanceIP(instance)
+ self.devices.append(
+ avd.AndroidVirtualDevice(ip=ip, instance_name=instance))
+
+ def WaitForBoot(self):
+ """Waits for all devices to boot up.
+
+ Returns:
+ A dictionary that contains all the failures.
+ The key is the name of the instance that fails to boot,
+ and the value is an errors.DeviceBootError object.
+ """
+ failures = {}
+ for device in self._devices:
+ try:
+ self._compute_client.WaitForBoot(device.instance_name)
+ except errors.DeviceBootError as e:
+ failures[device.instance_name] = e
+ return failures
+
+ @property
+ def devices(self):
+ """Returns a list of devices in the pool.
+
+ Returns:
+ A list of devices in the pool.
+ """
+ return self._devices
def CreateDevices(command, cfg, device_factory, num):
- """Create a set of devices using the given factory.
+ """Create a set of devices using the given factory.
- Args:
- command: The name of the command, used for reporting.
- cfg: An AcloudConfig instance.
- device_factory: A factory capable of producing a single device.
- num: The number of devices to create.
+ Args:
+ command: The name of the command, used for reporting.
+ cfg: An AcloudConfig instance.
+ device_factory: A factory capable of producing a single device.
+ num: The number of devices to create.
- Returns:
- A Report instance.
- """
- reporter = report.Report(command=command)
- try:
- CreateSshKeyPairIfNecessary(cfg)
- device_pool = DevicePool(device_factory)
- device_pool.CreateDevices(num)
- failures = device_pool.WaitForBoot()
- # Write result to report.
- for device in device_pool.devices:
- device_dict = {"ip": device.ip, "instance_name": device.instance_name}
- if device.instance_name in failures:
- reporter.AddData(key="devices_failing_boot", value=device_dict)
- reporter.AddError(str(failures[device.instance_name]))
- else:
- reporter.AddData(key="devices", value=device_dict)
- if failures:
- reporter.SetStatus(report.Status.BOOT_FAIL)
- else:
- reporter.SetStatus(report.Status.SUCCESS)
- except errors.DriverError as e:
- reporter.AddError(str(e))
- reporter.SetStatus(report.Status.FAIL)
- return reporter
+ Returns:
+ A Report instance.
+ """
+ reporter = report.Report(command=command)
+ try:
+ CreateSshKeyPairIfNecessary(cfg)
+ device_pool = DevicePool(device_factory)
+ device_pool.CreateDevices(num)
+ failures = device_pool.WaitForBoot()
+ # Write result to report.
+ for device in device_pool.devices:
+ device_dict = {
+ "ip": device.ip,
+ "instance_name": device.instance_name
+ }
+ if device.instance_name in failures:
+ reporter.AddData(key="devices_failing_boot", value=device_dict)
+ reporter.AddError(str(failures[device.instance_name]))
+ else:
+ reporter.AddData(key="devices", value=device_dict)
+ if failures:
+ reporter.SetStatus(report.Status.BOOT_FAIL)
+ else:
+ reporter.SetStatus(report.Status.SUCCESS)
+ except errors.DriverError as e:
+ reporter.AddError(str(e))
+ reporter.SetStatus(report.Status.FAIL)
+ return reporter
diff --git a/public/actions/common_operations_test.py b/public/actions/common_operations_test.py
index 87a1227..91fcbd6 100644
--- a/public/actions/common_operations_test.py
+++ b/public/actions/common_operations_test.py
@@ -13,16 +13,15 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
"""Tests for acloud.public.actions.common_operations."""
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function
+import unittest
import mock
-import unittest
from acloud.internal.lib import android_build_client
from acloud.internal.lib import android_compute_client
from acloud.internal.lib import auth
@@ -32,61 +31,67 @@
class CommonOperationsTest(driver_test_lib.BaseDriverTest):
+ """Test Common Operations."""
+ IP = "127.0.0.1"
+ INSTANCE = "fake-instance"
+ CMD = "test-cmd"
- IP = "127.0.0.1"
- INSTANCE = "fake-instance"
- CMD = "test-cmd"
+ def setUp(self):
+ """Set up the test."""
+ super(CommonOperationsTest, self).setUp()
+ self.build_client = mock.MagicMock()
+ self.device_factory = mock.MagicMock()
+ self.Patch(
+ android_build_client,
+ "AndroidBuildClient",
+ return_value=self.build_client)
+ self.compute_client = mock.MagicMock()
+ self.Patch(
+ android_compute_client,
+ "AndroidComputeClient",
+ return_value=self.compute_client)
+ self.Patch(auth, "CreateCredentials", return_value=mock.MagicMock())
+ self.Patch(self.compute_client, "GetInstanceIP", return_value=self.IP)
+ self.Patch(
+ self.device_factory, "CreateInstance", return_value=self.INSTANCE)
+ self.Patch(
+ self.device_factory,
+ "GetComputeClient",
+ return_value=self.compute_client)
- def setUp(self):
- """Set up the test."""
- super(CommonOperationsTest, self).setUp()
- self.build_client = mock.MagicMock()
- self.device_factory = mock.MagicMock()
- self.Patch(
- android_build_client,
- "AndroidBuildClient",
- return_value=self.build_client)
- self.compute_client = mock.MagicMock()
- self.Patch(
- android_compute_client,
- "AndroidComputeClient",
- return_value=self.compute_client)
- self.Patch(auth, "CreateCredentials", return_value=mock.MagicMock())
- self.Patch(self.compute_client, "GetInstanceIP", return_value=self.IP)
- self.Patch(
- self.device_factory, "CreateInstance", return_value=self.INSTANCE)
- self.Patch(self.device_factory, "GetComputeClient",
- return_value=self.compute_client)
+ @staticmethod
+ def _CreateCfg():
+ """A helper method that creates a mock configuration object."""
+ cfg = mock.MagicMock()
+ cfg.service_account_name = "fake@service.com"
+ cfg.service_account_private_key_path = "/fake/path/to/key"
+ cfg.zone = "fake_zone"
+ cfg.disk_image_name = "fake_image.tar.gz"
+ cfg.disk_image_mime_type = "fake/type"
+ cfg.ssh_private_key_path = ""
+ cfg.ssh_public_key_path = ""
+ return cfg
- def _CreateCfg(self):
- """A helper method that creates a mock configuration object."""
- cfg = mock.MagicMock()
- cfg.service_account_name = "fake@service.com"
- cfg.service_account_private_key_path = "/fake/path/to/key"
- cfg.zone = "fake_zone"
- cfg.disk_image_name = "fake_image.tar.gz"
- cfg.disk_image_mime_type = "fake/type"
- cfg.ssh_private_key_path = ""
- cfg.ssh_public_key_path = ""
- return cfg
+ def testDevicePoolCreateDevices(self):
+ """Test Device Pool Create Devices."""
+ pool = common_operations.DevicePool(self.device_factory)
+ pool.CreateDevices(5)
+ self.assertEqual(self.device_factory.CreateInstance.call_count, 5)
+ self.assertEqual(len(pool.devices), 5)
- def testDevicePoolCreateDevices(self):
- pool = common_operations.DevicePool(self.device_factory)
- pool.CreateDevices(5)
- self.assertEqual(self.device_factory.CreateInstance.call_count, 5)
- self.assertEqual(len(pool.devices), 5)
-
- def testCreateDevices(self):
- cfg = self._CreateCfg()
- r = common_operations.CreateDevices(self.CMD, cfg, self.device_factory, 1)
- self.assertEqual(r.command, self.CMD)
- self.assertEqual(r.status, report.Status.SUCCESS)
- self.assertEqual(
- r.data, {"devices": [{
- "ip": self.IP,
- "instance_name": self.INSTANCE
- }]})
+ def testCreateDevices(self):
+ """Test Create Devices."""
+ cfg = self._CreateCfg()
+ _report = common_operations.CreateDevices(self.CMD, cfg, self.device_factory, 1)
+ self.assertEqual(_report.command, self.CMD)
+ self.assertEqual(_report.status, report.Status.SUCCESS)
+ self.assertEqual(
+ _report.data,
+ {"devices": [{
+ "ip": self.IP,
+ "instance_name": self.INSTANCE
+ }]})
if __name__ == "__main__":
- unittest.main()
+ unittest.main()
diff --git a/public/avd.py b/public/avd.py
index d2e9c8d..9df3df0 100755
--- a/public/avd.py
+++ b/public/avd.py
@@ -13,10 +13,9 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
"""This module defines an AVD instance.
-TODO(fdeng):
+TODO:
The current implementation only initialize an object
with IP and instance name. A complete implementation
will include the following features.
@@ -50,9 +49,10 @@
@property
def ip(self):
+ """Getter of _ip."""
if not self._ip:
- raise ValueError("IP of instance %s is unknown yet." %
- self._instance_name)
+ raise ValueError(
+ "IP of instance %s is unknown yet." % self._instance_name)
return self._ip
@ip.setter
@@ -61,6 +61,7 @@
@property
def instance_name(self):
+ """Getter of _instance_name."""
return self._instance_name
def __str__(self):