[GitHub] [qpid-dispatch] jiridanek opened a new pull request #1058: DISPATCH-1974 Remove flake8 suppressions and fix import issues F401 and F403

[GitBox]

jiridanek opened a new pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[GitBox]

jiridanek commented on a change in pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058#discussion_r584110392



##########
File path: python/qpid_dispatch_internal/display_name/display_name.py
##########
@@ -29,14 +29,10 @@
 from __future__ import absolute_import
 from __future__ import print_function
 
-
+import json
 import traceback
-from traceback import format_exc
 
-from ..router.message import Message
-
-from ..dispatch import IoAdapter, LogAdapter, LOG_INFO, LOG_ERROR, LOG_TRACE, LOG_STACK_LIMIT
-import json
+from qpid_dispatch_internal import dispatch

Review comment:
       I admit going this far in this particular case is somewhat gratuitous, but referring to imported names with `import module; module.name` imo generally works better overall than `from module import name; name`.

##########
File path: tests/system_tests_topology_disposition.py.in
##########
@@ -423,12 +417,6 @@ class TopologyDispositionTests (TestCase):
         if self.skip[name] :
             self.skipTest("Test skipped during development.")
 
-        try:
-            import ast

Review comment:
       ast is available everywhere since Python 2.6

##########
File path: tests/system_tests_tcp_adaptor.py
##########
@@ -61,7 +55,7 @@ class TCP_echo_server(object):
 DISABLE_SELECTOR_TESTS = False
 DISABLE_SELECTOR_REASON = ''
 try:
-    import selectors
+    import selectors  # noqa F401: 'selectors' imported but unused

Review comment:
       btw, this is the way to do line-suppressions for flake8; afaik dispatch did not yet have any such

##########
File path: tests/tox.ini.in
##########
@@ -96,10 +96,6 @@ ignore =
     H301,
     # H306: imports not in alphabetical order
     H306,
-    # imported but unused
-    F401,
-    # 'from proton import *' used; unable to detect undefined names
-    F403,

Review comment:
       here goes the main part, everything else is just to fix the warnings that creep up when this here is removed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[GitBox]

codecov-io commented on pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058#issuecomment-787066203


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=h1) Report
   > Merging [#1058](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=desc) (00c3183) into [master](https://codecov.io/gh/apache/qpid-dispatch/commit/a0451227c7480e89b0892ffa962e77d157ffc370?el=desc) (a045122) will **decrease** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1058      +/-   ##
   ==========================================
   - Coverage   82.33%   82.33%   -0.01%    
   ==========================================
     Files         111      111              
     Lines       27472    27472              
   ==========================================
   - Hits        22620    22619       -1    
   - Misses       4852     4853       +1    
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `92.64% <0.00%> (-0.40%)` | :arrow_down: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `92.45% <0.00%> (-0.22%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL21lc3NhZ2UuYw==) | `87.07% <0.00%> (-0.08%)` | :arrow_down: |
   | [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `84.79% <0.00%> (ø)` | |
   | [src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci5j) | `86.78% <0.00%> (+0.10%)` | :arrow_up: |
   | [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1058/diff?src=pr&el=tree#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=) | `93.12% <0.00%> (+0.38%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=footer). Last update [a045122...00c3183](https://codecov.io/gh/apache/qpid-dispatch/pull/1058?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[GitBox]

ChugR commented on a change in pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058#discussion_r586511406



##########
File path: python/qpid_dispatch_internal/display_name/display_name.py
##########
@@ -29,14 +29,10 @@
 from __future__ import absolute_import
 from __future__ import print_function
 
-
+import json
 import traceback
-from traceback import format_exc
 
-from ..router.message import Message
-
-from ..dispatch import IoAdapter, LogAdapter, LOG_INFO, LOG_ERROR, LOG_TRACE, LOG_STACK_LIMIT
-import json
+from qpid_dispatch_internal import dispatch

Review comment:
       agreed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[GitBox]

jiridanek commented on a change in pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058#discussion_r586529772



##########
File path: tests/system_tests_tcp_adaptor.py
##########
@@ -61,7 +55,7 @@ class TCP_echo_server(object):
 DISABLE_SELECTOR_TESTS = False
 DISABLE_SELECTOR_REASON = ''
 try:
-    import selectors
+    import selectors  # noqa F401: 'selectors' imported but unused

Review comment:
       I'll update the comment to actually explain what's this about.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[GitBox]

jiridanek merged pull request #1058:
URL: https://github.com/apache/qpid-dispatch/pull/1058


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]