Skip to content

ref: Introduce linter for proper naming conventions #636

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

Merged
merged 2 commits into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .flake8
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
[flake8]
ignore =
E203, E266, E501, W503, E402, E731, C901, B950, B011,
B014 // does not apply to Python 2
E203, // Handled by black (Whitespace before ':' -- handled by black)
E266, // Handled by black (Too many leading '#' for block comment)
E501, // Handled by black (Line too long)
W503, // Handled by black (Line break occured before a binary operator)
E402, // Sometimes not possible due to execution order (Module level import is not at top of file)
E731, // I don't care (Do not assign a lambda expression, use a def)
C901, // I don't care (Function is too complex)
B950, // Handled by black (Line too long by flake8-bugbear)
B011, // I don't care (Do not call assert False)
B014, // does not apply to Python 2 (redundant exception types by flake8-bugbear)
N812, // I don't care (Lowercase imported as non-lowercase by pep8-naming)
N804 // is a worse version of and conflicts with B902 (first argument of a classmethod should be named cls)
max-line-length = 80
max-complexity = 18
select = B,C,E,F,W,T4,B9
select = N,B,C,E,F,W,T4,B9
exclude=checkouts,lol*,.tox
1 change: 1 addition & 0 deletions linter-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ flake8
flake8-import-order
mypy==0.761
flake8-bugbear>=19.8.0
pep8-naming
4 changes: 2 additions & 2 deletions sentry_sdk/_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ def reraise(tp, value, tb=None):

def with_metaclass(meta, *bases):
# type: (Any, *Any) -> Any
class metaclass(type):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might not be remembering well, but I thought class metaclass was common practice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't know. Are you saying that metaclasses are supposed to be snakecase?

class MetaClass(type):
def __new__(metacls, name, this_bases, d):
# type: (Any, Any, Any, Any) -> Any
return meta(name, bases, d)

return type.__new__(metaclass, "temporary_class", (), {})
return type.__new__(MetaClass, "temporary_class", (), {})


def check_thread_support():
Expand Down
2 changes: 1 addition & 1 deletion sentry_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ def __exit__(self, exc_type, exc_value, tb):
# Use `ClientConstructor` to define the argument types of `init` and
# `Dict[str, Any]` to tell static analyzers about the return type.

class get_options(ClientConstructor, Dict[str, Any]):
class get_options(ClientConstructor, Dict[str, Any]): # noqa: N801
pass

class Client(ClientConstructor, _Client):
Expand Down
2 changes: 1 addition & 1 deletion sentry_sdk/hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _init(*args, **kwargs):
# Use `ClientConstructor` to define the argument types of `init` and
# `ContextManager[Any]` to tell static analyzers about the return type.

class init(ClientConstructor, ContextManager[Any]):
class init(ClientConstructor, ContextManager[Any]): # noqa: N801
pass


Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
raise DidNotEnable("Bottle not installed")


TRANSACTION_STYLE_VALUES = ("endpoint", "url")


class BottleIntegration(Integration):
identifier = "bottle"

Expand All @@ -42,7 +45,6 @@ class BottleIntegration(Integration):
def __init__(self, transaction_style="endpoint"):
# type: (str) -> None

TRANSACTION_STYLE_VALUES = ("endpoint", "url")
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ def is_authenticated(request_user):
return request_user.is_authenticated


TRANSACTION_STYLE_VALUES = ("function_name", "url")


class DjangoIntegration(Integration):
identifier = "django"

Expand All @@ -79,7 +82,6 @@ class DjangoIntegration(Integration):

def __init__(self, transaction_style="url", middleware_spans=True):
# type: (str, bool) -> None
TRANSACTION_STYLE_VALUES = ("function_name", "url")
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/falcon.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,16 @@ def process_request(self, req, resp, *args, **kwargs):
scope.add_event_processor(_make_request_event_processor(req, integration))


TRANSACTION_STYLE_VALUES = ("uri_template", "path")


class FalconIntegration(Integration):
identifier = "falcon"

transaction_style = None

def __init__(self, transaction_style="uri_template"):
# type: (str) -> None
TRANSACTION_STYLE_VALUES = ("uri_template", "path")
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,16 @@
raise DidNotEnable("Flask is not installed")


TRANSACTION_STYLE_VALUES = ("endpoint", "url")


class FlaskIntegration(Integration):
identifier = "flask"

transaction_style = None

def __init__(self, transaction_style="endpoint"):
# type: (str) -> None
TRANSACTION_STYLE_VALUES = ("endpoint", "url")
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/pyramid.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ def authenticated_userid(request):
from pyramid.security import authenticated_userid # type: ignore


TRANSACTION_STYLE_VALUES = ("route_name", "route_pattern")


class PyramidIntegration(Integration):
identifier = "pyramid"

transaction_style = None

def __init__(self, transaction_style="route_name"):
# type: (str) -> None
TRANSACTION_STYLE_VALUES = ("route_name", "route_pattern")
if transaction_style not in TRANSACTION_STYLE_VALUES:
raise ValueError(
"Invalid value for transaction_style: %s (must be in %s)"
Expand Down
84 changes: 43 additions & 41 deletions sentry_sdk/integrations/spark/spark_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def _set_app_properties():
"""
from pyspark import SparkContext

sparkContext = SparkContext._active_spark_context
if sparkContext:
sparkContext.setLocalProperty("sentry_app_name", sparkContext.appName)
sparkContext.setLocalProperty(
"sentry_application_id", sparkContext.applicationId
spark_context = SparkContext._active_spark_context
if spark_context:
spark_context.setLocalProperty("sentry_app_name", spark_context.appName)
spark_context.setLocalProperty(
"sentry_application_id", spark_context.applicationId
)


Expand Down Expand Up @@ -106,99 +106,101 @@ def process_event(event, hint):


class SparkListener(object):
def onApplicationEnd(self, applicationEnd):
def onApplicationEnd(self, applicationEnd): # noqa: N802,N803
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe apply this to the whole class or module/file?
Doing it per method is very noisy :(

Also could we have a note like "# noqa: ...; ignore camelCase in methods names and attributes because this code is inherited from somewhere..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how to specify it on a class level. I would like to avoid doing it on a module level for sure asssuming we do not restructure the modules.

# type: (Any) -> None
pass

def onApplicationStart(self, applicationStart):
def onApplicationStart(self, applicationStart): # noqa: N802,N803
# type: (Any) -> None
pass

def onBlockManagerAdded(self, blockManagerAdded):
def onBlockManagerAdded(self, blockManagerAdded): # noqa: N802,N803
# type: (Any) -> None
pass

def onBlockManagerRemoved(self, blockManagerRemoved):
def onBlockManagerRemoved(self, blockManagerRemoved): # noqa: N802,N803
# type: (Any) -> None
pass

def onBlockUpdated(self, blockUpdated):
def onBlockUpdated(self, blockUpdated): # noqa: N802,N803
# type: (Any) -> None
pass

def onEnvironmentUpdate(self, environmentUpdate):
def onEnvironmentUpdate(self, environmentUpdate): # noqa: N802,N803
# type: (Any) -> None
pass

def onExecutorAdded(self, executorAdded):
def onExecutorAdded(self, executorAdded): # noqa: N802,N803
# type: (Any) -> None
pass

def onExecutorBlacklisted(self, executorBlacklisted):
def onExecutorBlacklisted(self, executorBlacklisted): # noqa: N802,N803
# type: (Any) -> None
pass

def onExecutorBlacklistedForStage(self, executorBlacklistedForStage):
def onExecutorBlacklistedForStage( # noqa: N802
self, executorBlacklistedForStage # noqa: N803
):
# type: (Any) -> None
pass

def onExecutorMetricsUpdate(self, executorMetricsUpdate):
def onExecutorMetricsUpdate(self, executorMetricsUpdate): # noqa: N802,N803
# type: (Any) -> None
pass

def onExecutorRemoved(self, executorRemoved):
def onExecutorRemoved(self, executorRemoved): # noqa: N802,N803
# type: (Any) -> None
pass

def onJobEnd(self, jobEnd):
def onJobEnd(self, jobEnd): # noqa: N802,N803
# type: (Any) -> None
pass

def onJobStart(self, jobStart):
def onJobStart(self, jobStart): # noqa: N802,N803
# type: (Any) -> None
pass

def onNodeBlacklisted(self, nodeBlacklisted):
def onNodeBlacklisted(self, nodeBlacklisted): # noqa: N802,N803
# type: (Any) -> None
pass

def onNodeBlacklistedForStage(self, nodeBlacklistedForStage):
def onNodeBlacklistedForStage(self, nodeBlacklistedForStage): # noqa: N802,N803
# type: (Any) -> None
pass

def onNodeUnblacklisted(self, nodeUnblacklisted):
def onNodeUnblacklisted(self, nodeUnblacklisted): # noqa: N802,N803
# type: (Any) -> None
pass

def onOtherEvent(self, event):
def onOtherEvent(self, event): # noqa: N802,N803
# type: (Any) -> None
pass

def onSpeculativeTaskSubmitted(self, speculativeTask):
def onSpeculativeTaskSubmitted(self, speculativeTask): # noqa: N802,N803
# type: (Any) -> None
pass

def onStageCompleted(self, stageCompleted):
def onStageCompleted(self, stageCompleted): # noqa: N802,N803
# type: (Any) -> None
pass

def onStageSubmitted(self, stageSubmitted):
def onStageSubmitted(self, stageSubmitted): # noqa: N802,N803
# type: (Any) -> None
pass

def onTaskEnd(self, taskEnd):
def onTaskEnd(self, taskEnd): # noqa: N802,N803
# type: (Any) -> None
pass

def onTaskGettingResult(self, taskGettingResult):
def onTaskGettingResult(self, taskGettingResult): # noqa: N802,N803
# type: (Any) -> None
pass

def onTaskStart(self, taskStart):
def onTaskStart(self, taskStart): # noqa: N802,N803
# type: (Any) -> None
pass

def onUnpersistRDD(self, unpersistRDD):
def onUnpersistRDD(self, unpersistRDD): # noqa: N802,N803
# type: (Any) -> None
pass

Expand All @@ -211,13 +213,13 @@ def __init__(self):
# type: () -> None
self.hub = Hub.current

def onJobStart(self, jobStart):
def onJobStart(self, jobStart): # noqa: N802,N803
# type: (Any) -> None
message = "Job {} Started".format(jobStart.jobId())
self.hub.add_breadcrumb(level="info", message=message)
_set_app_properties()

def onJobEnd(self, jobEnd):
def onJobEnd(self, jobEnd): # noqa: N802,N803
# type: (Any) -> None
level = ""
message = ""
Expand All @@ -232,30 +234,30 @@ def onJobEnd(self, jobEnd):

self.hub.add_breadcrumb(level=level, message=message, data=data)

def onStageSubmitted(self, stageSubmitted):
def onStageSubmitted(self, stageSubmitted): # noqa: N802,N803
# type: (Any) -> None
stageInfo = stageSubmitted.stageInfo()
message = "Stage {} Submitted".format(stageInfo.stageId())
data = {"attemptId": stageInfo.attemptId(), "name": stageInfo.name()}
stage_info = stageSubmitted.stageInfo()
message = "Stage {} Submitted".format(stage_info.stageId())
data = {"attemptId": stage_info.attemptId(), "name": stage_info.name()}
self.hub.add_breadcrumb(level="info", message=message, data=data)
_set_app_properties()

def onStageCompleted(self, stageCompleted):
def onStageCompleted(self, stageCompleted): # noqa: N802,N803
# type: (Any) -> None
from py4j.protocol import Py4JJavaError # type: ignore

stageInfo = stageCompleted.stageInfo()
stage_info = stageCompleted.stageInfo()
message = ""
level = ""
data = {"attemptId": stageInfo.attemptId(), "name": stageInfo.name()}
data = {"attemptId": stage_info.attemptId(), "name": stage_info.name()}

# Have to Try Except because stageInfo.failureReason() is typed with Scala Option
try:
data["reason"] = stageInfo.failureReason().get()
message = "Stage {} Failed".format(stageInfo.stageId())
data["reason"] = stage_info.failureReason().get()
message = "Stage {} Failed".format(stage_info.stageId())
level = "warning"
except Py4JJavaError:
message = "Stage {} Completed".format(stageInfo.stageId())
message = "Stage {} Completed".format(stage_info.stageId())
level = "info"

self.hub.add_breadcrumb(level=level, message=message, data=data)
24 changes: 12 additions & 12 deletions sentry_sdk/integrations/spark/spark_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,31 +76,31 @@ def process_event(event, hint):
# type: (Event, Hint) -> Optional[Event]
with capture_internal_exceptions():
integration = Hub.current.get_integration(SparkWorkerIntegration)
taskContext = TaskContext.get()
task_context = TaskContext.get()

if integration is None or taskContext is None:
if integration is None or task_context is None:
return event

event.setdefault("tags", {}).setdefault(
"stageId", taskContext.stageId()
"stageId", task_context.stageId()
)
event["tags"].setdefault("partitionId", taskContext.partitionId())
event["tags"].setdefault("attemptNumber", taskContext.attemptNumber())
event["tags"].setdefault("taskAttemptId", taskContext.taskAttemptId())
event["tags"].setdefault("partitionId", task_context.partitionId())
event["tags"].setdefault("attemptNumber", task_context.attemptNumber())
event["tags"].setdefault("taskAttemptId", task_context.taskAttemptId())

if taskContext._localProperties:
if "sentry_app_name" in taskContext._localProperties:
if task_context._localProperties:
if "sentry_app_name" in task_context._localProperties:
event["tags"].setdefault(
"app_name", taskContext._localProperties["sentry_app_name"]
"app_name", task_context._localProperties["sentry_app_name"]
)
event["tags"].setdefault(
"application_id",
taskContext._localProperties["sentry_application_id"],
task_context._localProperties["sentry_application_id"],
)

if "callSite.short" in taskContext._localProperties:
if "callSite.short" in task_context._localProperties:
event.setdefault("extra", {}).setdefault(
"callSite", taskContext._localProperties["callSite.short"]
"callSite", task_context._localProperties["callSite.short"]
)

return event
Expand Down
Loading