copr / copr

Clone

#324 [backend] resolving pylint warnings - Wrong hanging or continued indentation
Merged 5 years ago by msuchy. Opened 5 years ago by pjunak.
copr/ pjunak/copr pylint  into  master

file modified
+1 -1 
@@ -81,7 +81,7 @@ 
 

                                        .format(int(time.time() - get_task_init_time)))
 

              try:
 

                  tasks = get("{0}/backend/pending-jobs/".format(self.opts.frontend_base_url),
 

-                            auth=("user", self.opts.frontend_auth)).json()
 

+                             auth=("user", self.opts.frontend_auth)).json()
 

  

              except (RequestException, ValueError) as error:
 

                  self.log.exception("Retrieving build jobs from {} failed with error: {}"

file modified
+1 -1 
@@ -224,7 +224,7 @@ 
 

                      # garbage collector.
 

                      self.vm_manager.start_vm_termination(self.vm.vm_name)
 

                      self.frontend_client.reschedule_build(
 

-                             job.build_id, job.task_id, job.chroot)
 

+                         job.build_id, job.task_id, job.chroot)
 

                      raise VmError("SSH connection issue, build rescheduled")
 

  

                  except: # programmer's failure

file modified
+3 -3 
@@ -204,9 +204,9 @@ 
 

          base_url = "/".join([self.opts.results_baseurl, self.job.project_owner,
 

                               self.job.project_name, self.job.chroot])
 

          self.log.info("Createrepo:: owner:  {}; project: {}; "
 

-                        "front url: {}; path: {}; base_url: {}"
 

-                        .format(self.job.project_owner, self.job.project_name,
 

-                                self.opts.frontend_base_url, self.chroot_dir, base_url))
 

+                       "front url: {}; path: {}; base_url: {}"
 

+                       .format(self.job.project_owner, self.job.project_name,
 

+                               self.opts.frontend_base_url, self.chroot_dir, base_url))
 

  

          try:
 

              createrepo(

file modified
+2 -2 
@@ -117,7 +117,7 @@ 
 

          if not self.opts.builder_deprecated:
 

              return 'copr-rpmbuild --verbose --drop-resultdir '\
 

                     '--build-id {build_id} --chroot {chroot} --detached'.format(
 

-                 build_id=self.job.build_id, chroot=self.job.chroot)
 

+                        build_id=self.job.build_id, chroot=self.job.chroot)
 

  

          template = 'copr-builder --config {config} --copr {copr} ' \
 

                   + '--package {package} --revision {revision} ' \
 
@@ -126,7 +126,7 @@ 
 

  

          if self.module_dist_tag:
 

              template += '--define {0} '.format(
 

-                     pipes.quote('dist ' + self.module_dist_tag))
 

+                 pipes.quote('dist ' + self.module_dist_tag))
 

  

          # Repo name like <user/group>/<copr>/<package>
 

          git_repo_path = self.job.git_repo.split('/')

file modified
+4 -4 
@@ -176,10 +176,10 @@ 
 

                  ssl_key, ssl_crt, cacert))
 

  

              self.conn.set_ssl(
 

-                     for_hosts=hosts,
 

-                     key_file=ssl_key,
 

-                     cert_file=ssl_crt,
 

-                     ca_certs=cacert
 

+                 for_hosts=hosts,
 

+                 key_file=ssl_key,
 

+                 cert_file=ssl_crt,
 

+                 ca_certs=cacert
 

              )
 

  

          self.connect()

Examples:
C:179, 0: Wrong hanging indentation (remove 4 spaces).
for_hosts=hosts,
| ^ (bad-continuation)
C: 84, 0: Wrong continued indentation (add 1 space).
auth=("user", self.opts.frontend_auth)).json()
^| (bad-continuation)
I formatted the text accordingly to Pylint in all instances.

(\ not needed. Just use ( at the end of the line.

I mostly did what Pylint told me, except in builder.py where the problem was caused by missing "\" at the end of both problematic lines. I added the missing slashes and I edited the code to look clearer.

Thanks! But these extra slashes at the end of lines '(/' should not be there because they are not needed. Just '(' at the end of the lines is fine (e.g. '.format(' at the end of a line is totally fine).

I mostly did what Pylint told me, except in builder.py where the problem was caused by missing "\" at the end of both problematic lines. I added the missing slashes and I edited the code to look clearer.

Thanks! But these extra slashes at the end of lines '(/' should not be there because they are not needed. Just '(' at the end of the lines is fine (e.g. '.format(' at the end of a line is totally fine).

Ok, I can remove the slashes and disable the warnings. Are you satisfied with how I formatted the lines?

If I did a poor job, please, tell me what should I do better.

Please don't excuse like that ;-) your work is appreciated and it's always essential to to do the review.

Ok, I can remove the slashes and disable the warnings.

I agree with @clime that those backslashes shouldn't be needed.
Edited 5 years ago by praiskup

I mostly did what Pylint told me, except in builder.py where the problem was caused by missing "\" at the end of both problematic lines. I added the missing slashes and I edited the code to look clearer.
Thanks! But these extra slashes at the end of lines '(/' should not be there because they are not needed. Just '(' at the end of the lines is fine (e.g. '.format(' at the end of a line is totally fine).

Ok, I can remove the slashes and disable the warnings. Are you satisfied with how I formatted the lines?

Yup, formatting looks better than before.

This helped me:

-                build_id=self.job.build_id, chroot=self.job.chroot)
+                       build_id=self.job.build_id, chroot=self.job.chroot)

rebased onto a71bfe40ae77e0cb9bb413199f5f5d6e9341189a
5 years ago

I have removed the unnecessary slashes in builder.py and disabled Pylint warnings.

Looks good to me. Thanks!

:thumbsup:

no need to blacklist the issue here, it's enough to move it a bit right

-1 for the # pylint: disable=bad-continuation

Ok, I need to know which version you all prefer:

    if not self.opts.builder_deprecated:
        return 'copr-rpmbuild --verbose --drop-resultdir '\
               '--build-id {build_id} --chroot {chroot} --detached'.format(
               build_id=self.job.build_id, chroot=self.job.chroot) # pylint: disable=bad-continuation

or:

    if not self.opts.builder_deprecated:
        return 'copr-rpmbuild --verbose --drop-resultdir '\
               '--build-id {build_id} --chroot {chroot} --detached'.format(
                   build_id=self.job.build_id, chroot=self.job.chroot)

+1 for the second version

2 new commits added

  • [createrepo, helpers, daemons, manager] resolving pylint warnings - no
  • [builder] resolving pylint warning - line too long
5 years ago

Note the "semantics" of tagging commit messages... Usually you add [backend] into commit message if you edit code related to backend, it is not related to classes (manager, etc.).

1 new commit added

  • [msgbus, build_dispatcher, worker, __init__,builder] resolving pylint warnings - Wrong hanging or continued indentation
5 years ago

rebased onto 12b215764edeca9d9602c25c6f1c87096e5e90ec
5 years ago

rebased onto 8187c8b
5 years ago

Note the "semantics" of tagging commit messages... Usually you add [backend] into commit message if you edit code related to backend, it is not related to classes (manager, etc.).

Thanks.
I removed the blacklistings and added the spaces.

Looks good to me.

Nice job. Thank you.

+1

Pull-Request has been merged by msuchy
5 years ago
Metadata
None
No Tags
Changes Summary 5
+1 -1
file changed
backend/backend/daemons/build_dispatcher.py
+1 -1
file changed
backend/backend/daemons/worker.py
+3 -3
file changed
backend/backend/mockremote/__init__.py
+2 -2
file changed
backend/backend/mockremote/builder.py
+4 -4
file changed
backend/backend/msgbus.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.