#327 [backend] resolving pylint warnings - wrong import position and order, reimported, unused import
Merged 5 years ago by clime. Opened 5 years ago by pjunak.
copr/ pjunak/copr pylint  into  master

file modified
+4 -5
@@ -3,18 +3,17 @@ 

  import os.path

  import shutil

  import time

- import glob

  import traceback

  import base64

- import modulemd

- import tempfile

  

- from munch import Munch

  from distutils.dir_util import copy_tree

  from distutils.errors import DistutilsFileError

+ from urllib.request import urlretrieve

  from copr.exceptions import CoprRequestException

  from requests import RequestException

- from urllib.request import urlretrieve

+ from munch import Munch

+ 

+ import modulemd

  

  from .sign import create_user_keys, CoprKeygenRequestError

  from .createrepo import createrepo

file modified
+1 -1
@@ -1,4 +1,5 @@ 

  import os

+ from logging import Formatter

  

  mockchain = "/usr/bin/mockchain"

  # rsync path
@@ -36,7 +37,6 @@ 

  

  LOG_PUB_SUB = "copr:backend:log:pubsub::"

  

- from logging import Formatter

  default_log_format = Formatter(

      '[%(asctime)s][%(levelname)6s][%(name)10s][%(filename)s:%(funcName)s:%(lineno)d] %(message)s')

  build_log_format = Formatter(

@@ -1,8 +1,8 @@ 

  import os

  from subprocess import Popen, PIPE

  

- from setproctitle import getproctitle, setproctitle

  from shlex import split

+ from setproctitle import getproctitle, setproctitle

  from lockfile import LockFile

  

  # todo: add logging here

@@ -1,6 +1,6 @@ 

  # coding: utf-8

  

- import json

+ import json #line 49 # pylint: disable=unused-import

  import time

  import multiprocessing

  from setproctitle import setproctitle

@@ -2,21 +2,15 @@ 

  

  import grp

  import pwd

- import signal

  import sys

- import time

- from collections import defaultdict

  

  import lockfile

  from daemon import DaemonContext

  from requests import RequestException

- from retask import ConnectionError

  from backend.frontend import FrontendClient

  

  from ..exceptions import CoprBackendError

  from ..helpers import BackendConfigReader, get_redis_logger

- from .build_dispatcher import BuildDispatcher

- from .action_dispatcher import ActionDispatcher

  

  

  class CoprBackend(object):

@@ -1,9 +1,11 @@ 

  # coding: utf-8

  

  import time

- import os

  import multiprocessing

- import json

+ import json #line 85 # pylint: disable=unused-import

+ 

+ from collections import defaultdict

+ 

  from setproctitle import setproctitle

  from requests import get, RequestException

  
@@ -16,7 +18,6 @@ 

  from ..constants import BuildStatus

  from .worker import Worker

  

- from collections import defaultdict

  

  class BuildDispatcher(multiprocessing.Process):

      """

@@ -1,11 +1,10 @@ 

  # coding: utf-8

  

- import json

- 

  from multiprocessing import Process

  import time

- from setproctitle import setproctitle

  import traceback

+ from setproctitle import setproctitle

+ 

  

  from ..vm_manage import VmStates

  from ..exceptions import VmSpawnLimitReached

@@ -1,5 +1,3 @@ 

- from datetime import datetime

- import json

  import os

  import time

  import gzip
@@ -9,13 +7,12 @@ 

  import glob

  from setproctitle import setproctitle

  

- from ..exceptions import MockRemoteError, CoprWorkerError, VmError, NoVmAvailable

- from ..job import BuildJob

+ from ..exceptions import MockRemoteError, CoprWorkerError, VmError

+ from ..job import BuildJob #line 151 (will be implemented?) #pylint: disable=unused-import

  from ..mockremote import MockRemote

  from ..constants import BuildStatus, build_log_format

- from ..helpers import register_build_result, get_redis_connection, get_redis_logger, \

+ from ..helpers import register_build_result, get_redis_logger, \

      local_file_logger, run_cmd

- from ..vm_manage import VmStates

  

  from ..msgbus import MsgBusStomp, MsgBusFedmsg

  from ..sshcmd import SSHConnectionError

file modified
+9 -5
@@ -8,29 +8,33 @@ 

  import time

  import types

  import glob

+ 

  import configparser

+ from configparser import ConfigParser

  

  from contextlib import contextmanager

  from operator import methodcaller

- from configparser import ConfigParser

  

  import traceback

  

  from datetime import datetime

+ 

+ import subprocess

+ 

  import pytz

  

+ import munch

  from munch import Munch

+ 

  from redis import StrictRedis

- from . import constants

  

  from copr.client import CoprClient

  from backend.constants import DEF_BUILD_USER, DEF_BUILD_TIMEOUT, DEF_CONSECUTIVE_FAILURE_THRESHOLD, \

      CONSECUTIVE_FAILURE_REDIS_KEY, default_log_format

  from backend.exceptions import CoprBackendError

  

- import subprocess

- import logging

- import munch

+ from . import constants

+ 

  

  def pyconffile(filename):

      """

@@ -1,20 +1,18 @@ 

  import os

  import pipes

- import socket

+ import socket #mentioned in TODO # pylint: disable=unused-import 

Not needed. It was in past needed by socket.gethostbyname() but it is gone now.

  from subprocess import Popen

- import time

  from urllib.parse import urlparse

- import glob

  

  from backend.vm_manage import PUBSUB_INTERRUPT_BUILDER

- from ..helpers import get_redis_connection, ensure_dir_exists

  

- from ..exceptions import BuilderError, RemoteCmdError, VmError

+ import modulemd

  

+ from ..helpers import get_redis_connection, ensure_dir_exists

+ from ..exceptions import BuilderError, RemoteCmdError, VmError

  from ..constants import rsync

- from ..sshcmd import SSHConnectionError, SSHConnection

+ from ..sshcmd import SSHConnection

  

- import modulemd

  

  

  class Builder(object):

@@ -1,9 +1,9 @@ 

  # coding: utf-8

  import json

  from multiprocessing import Process

- from setproctitle import setproctitle

  from threading import Thread

  import time

+ from setproctitle import setproctitle

  

  from backend.exceptions import VmDescriptorNotFound

  from backend.helpers import get_redis_logger

@@ -1,8 +1,8 @@ 

  # coding: utf-8

  

  from pprint import pformat

- from . import KEY_VM_INSTANCE

  from backend.exceptions import VmDescriptorNotFound

+ from . import KEY_VM_INSTANCE

  

  

  class VmDescriptor(object):

@@ -7,10 +7,10 @@ 

  

  from netaddr import IPAddress

  

- from ..ans_utils import run_ansible_playbook_cli

  from backend.helpers import get_redis_connection

  from backend.vm_manage import PUBSUB_MB, EventTopics

  from backend.vm_manage.executor import Executor

+ from ..ans_utils import run_ansible_playbook_cli

  from ..exceptions import CoprSpawnFailError

  from ..helpers import get_redis_logger

  from ..vm_manage import terminate

Examples:
Module backend.actions
C: 13, 0: standard import "from distutils.dir_util import copy_tree" should be placed before "from munch import Munch" (wrong-import-order)

Module backend.constants
C: 39, 0: Import "from logging import Formatter" should be placed at the top of the module (wrong-import-position)

Module backend.helpers
W: 36, 0: Reimport 'logging' (imported line 2) (reimported)

Module backend.actions
W: 6, 0: Unused import glob (unused-import)
W: 9, 0: Unused import tempfile (unused-import)

Looks good +1

I used to think that the only import order, that matter is import foo lines above from foo import bar, but thanks to this PR I learned, that

Imports should be grouped in the following order:

  1. standard library imports
  2. related third party imports
  3. local application/library specific imports

https://stackoverflow.com/q/22722976

and pylint actually checks it.

Actually, does this change help anything? import time is still below this PyPI package

rebased onto dbfa84e8baab384e11ac4c4dc9a86a44e363f4fa

5 years ago

rebased onto 4ad32b1

5 years ago

1 new commit added

  • [backend] resolving pylint warning / reimported
5 years ago

1 new commit added

  • [builder] resolving pylint warnings - unused import
5 years ago

Added multiple import related commits.

Why the pylint: disable=unused-import is needed on that many places?

Not needed. It was in past needed by socket.gethostbyname() but it is gone now.

@pjunak: still interested in finishing this? If so, please, remove the lines that have pylint: disable=unused-import on them. Those lines can be safely dropped.

Merging, additionally I will open a pull request to remove the lines with pylint: disable=unused-import.

Pull-Request has been merged by clime

5 years ago