Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flake8 cleanup of natlas-server #214

Merged
merged 12 commits into from Nov 4, 2019

Conversation

@0xdade
Copy link
Member

0xdade commented Nov 3, 2019

General style improvements in an effort to bring more consistency to the code base.

natlas-server/app/api/routes.py Outdated Show resolved Hide resolved
natlas-server/app/api/routes.py Outdated Show resolved Hide resolved
natlas-server/app/api/routes.py Outdated Show resolved Hide resolved
natlas-server/app/api/routes.py Outdated Show resolved Hide resolved
natlas-server/app/user/routes.py Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
@0xdade 0xdade requested a review from ajacques Nov 3, 2019
@natlas natlas deleted a comment from lgtm-com bot Nov 3, 2019
@natlas natlas deleted a comment from lgtm-com bot Nov 3, 2019
@natlas natlas deleted a comment from lgtm-com bot Nov 3, 2019
@natlas natlas deleted a comment from lgtm-com bot Nov 3, 2019
.flake8 Show resolved Hide resolved
natlas-server/app/api/routes.py Outdated Show resolved Hide resolved
natlas-server/app/api/utils.py Outdated Show resolved Hide resolved
@0xdade 0xdade force-pushed the 0xdade:main branch from 462f773 to d1ea278 Nov 4, 2019
@codeclimate

This comment has been minimized.

Copy link

codeclimate bot commented Nov 4, 2019

Code Climate has analyzed commit d1ea278 and detected 8 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 4
Duplication 2
Style 2

View more on Code Climate.

@lgtm-com

This comment has been minimized.

Copy link

lgtm-com bot commented Nov 4, 2019

This pull request fixes 7 alerts when merging d1ea278 into af6b1a9 - view on LGTM.com

fixed alerts:

  • 2 for First parameter of a method is not named 'self'
  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for 'import *' may pollute namespace
@natlas natlas deleted a comment from lgtm-com bot Nov 4, 2019
@ajacques

This comment has been minimized.

Copy link
Member

ajacques commented Nov 4, 2019

FYI (this applies to all error responses.) Sentry.io can only capture and auto report exceptions if you propagate up an exception. If there's something note-worthy that you want to track, either explicitly report it using the Python capture_exception or throw an exception back.

For example, we would want to report any exception that represents a bug or configuration error. I haven't looked through each Response type, but keep this in mind when we're wrapping up and creating Response objects.

https://docs.sentry.io/platforms/python/flask/

work["services_hash"] = current_app.current_services["sha256"]
work['scan_id'] = get_unique_scan_id()
work['status'] = 200
work['message'] = "Target: " + str(work['target'])

This comment has been minimized.

Copy link
@ajacques

ajacques Nov 4, 2019

Member

This message is a little weird. Why are passing back free text blobs back to the client?

This comment has been minimized.

Copy link
@0xdade

0xdade Nov 4, 2019

Author Member

This message isn't really necessary anymore, originally the agent networking code would print out any message it got from the server because I wrote it super poorly and didn't define the different types of errors that could occur. I added the status and message to a successful work request in an effort to make the json blobs more consistent about what fields were in them. But I think we can move away from this in the next iterations as we improve the networking code and properly define the interface.

@0xdade 0xdade merged commit fdef093 into natlas:main Nov 4, 2019
5 of 6 checks passed
5 of 6 checks passed
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python 7 fixed alerts
Details
ci/circleci: general-analysis Your tests passed on CircleCI!
Details
ci/dockercloud (/natlas-agent) Your tests passed in Docker Cloud
Details
ci/dockercloud (/natlas-server) Your tests passed in Docker Cloud
Details
codeclimate Approved by dade.
Details
def print_imports(msg, importList):
if len(importList) > 0:
print(msg)
print('\n'.join(importList))
Comment on lines +10 to +13

This comment has been minimized.

Copy link
@pryorda

pryorda Nov 4, 2019

Contributor

Stuff like this is going to make moving to a logger in the future easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.