Skip to content

Commit

Permalink
fix and enable several ruff linting rules (#10471)
Browse files Browse the repository at this point in the history
  • Loading branch information
thrau committed Mar 17, 2024
1 parent 676655b commit 628ed08
Show file tree
Hide file tree
Showing 40 changed files with 100 additions and 128 deletions.
2 changes: 1 addition & 1 deletion localstack/aws/handlers/cors.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _get_allowed_cors_ports() -> Set[int]:
Construct the list of allowed ports for CORS enforcement purposes
Defined as function to allow easier testing with monkeypatch of config values
"""
return set([host_and_port.port for host_and_port in config.GATEWAY_LISTEN])
return {host_and_port.port for host_and_port in config.GATEWAY_LISTEN}


_ALLOWED_INTERNAL_PORTS = _get_allowed_cors_ports()
Expand Down
4 changes: 1 addition & 3 deletions localstack/aws/handlers/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,7 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo

if exception := context.service_exception:
if isinstance(exception, ServiceException):
try:
exception.code
except AttributeError:
if not hasattr(exception, "code"):
# FIXME: we should set the exception attributes in the scaffold when we generate the exceptions.
# this is a workaround for now, since we are not doing that yet, and the attributes may be unset.
self._set_exception_attributes(context.operation, exception)
Expand Down
4 changes: 2 additions & 2 deletions localstack/aws/protocol/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,9 @@ def _parse_map(
parsed = {}
key_shape = shape.key
value_shape = shape.value
for key, value in value.items():
for key, val in value.items():
actual_key = self._parse_shape(request, key_shape, key, uri_params)
actual_value = self._parse_shape(request, value_shape, value, uri_params)
actual_value = self._parse_shape(request, value_shape, val, uri_params)
parsed[actual_key] = actual_value
return parsed

Expand Down
6 changes: 5 additions & 1 deletion localstack/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ def append(self, value: HostAndPort):

# if we add 0.0.0.0:<port> and already contain *:<port> then bind on
# 0.0.0.0
contained_ports = set(every.port for every in self)
contained_ports = {every.port for every in self}
if value.host == "0.0.0.0" and value.port in contained_ports:
for item in self:
if item.port == value.port:
Expand Down Expand Up @@ -1358,6 +1358,7 @@ def service_url(service_key, host=None, port=None):
"""@deprecated: Use `internal_service_url()` instead. We assume that most usages are
internal but really need to check and update each usage accordingly.""",
DeprecationWarning,
stacklevel=2,
)
return internal_service_url(host=host, port=port)

Expand All @@ -1369,6 +1370,7 @@ def service_port(service_key: str, external: bool = False) -> int:
"Deprecated: use `localstack_host().port` for external and `GATEWAY_LISTEN[0].port` for "
"internal use.",
DeprecationWarning,
stacklevel=2,
)
if external:
return LOCALSTACK_HOST.port
Expand All @@ -1384,6 +1386,7 @@ def get_edge_port_http():
for internal use. This function is also not needed anymore because we don't separate
between HTTP and HTTP ports anymore since LocalStack listens to both.""",
DeprecationWarning,
stacklevel=2,
)
return GATEWAY_LISTEN[0].port

Expand All @@ -1397,6 +1400,7 @@ def get_edge_url(localstack_hostname=None, protocol=None):
We assume that most usages are internal but really need to check and update each usage accordingly.
""",
DeprecationWarning,
stacklevel=2,
)
return internal_service_url(host=localstack_hostname, protocol=protocol)

Expand Down
2 changes: 1 addition & 1 deletion localstack/services/cloudformation/scaffolding/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def resolve_ref(schema: ResourceSchema, target: str) -> dict:
"""
Given a schema {"a": {"b": "c"}} and the ref "#/a/b" return "c"
"""
target_path = filter(None, map(lambda elem: elem.strip(), target.lstrip("#").split("/")))
target_path = filter(None, (elem.strip() for elem in target.lstrip("#").split("/")))

T = TypeVar("T")

Expand Down
4 changes: 2 additions & 2 deletions localstack/services/events/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ def events_handler_put_events(self):
if config.is_local_test_mode():
TEST_EVENTS_CACHE.extend(entries)

events = list(map(lambda event: {"event": event, "uuid": str(long_uid())}, entries))
events = [{"event": event, "uuid": str(long_uid())} for event in entries]

_dump_events_to_files(events)

Expand Down Expand Up @@ -714,7 +714,7 @@ def events_handler_put_events(self):

content = {
"FailedEntryCount": 0, # TODO: dynamically set proper value when refactoring
"Entries": list(map(lambda event: {"EventId": event["uuid"]}, events)),
"Entries": [{"EventId": event["uuid"]} for event in events],
}

self.response_headers.update(
Expand Down
2 changes: 1 addition & 1 deletion localstack/services/internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def on_get(self, request):
"stackName": "stack1",
"templateBody": "{}",
"errorMessage": "''",
"regions": json.dumps(sorted(list(get_valid_regions()))),
"regions": json.dumps(sorted(get_valid_regions())),
}

download_url = req_params.get("templateURL")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,10 @@


"""Map AWS Lambda architecture to Docker platform flags. Example: arm64 => linux/arm64"""
ARCHITECTURE_PLATFORM_MAPPING: dict[Architecture, DockerPlatform] = dict(
{
Architecture.x86_64: DockerPlatform.linux_amd64,
Architecture.arm64: DockerPlatform.linux_arm64,
}
)
ARCHITECTURE_PLATFORM_MAPPING: dict[Architecture, DockerPlatform] = {
Architecture.x86_64: DockerPlatform.linux_amd64,
Architecture.arm64: DockerPlatform.linux_arm64,
}


def docker_platform(lambda_architecture: Architecture) -> DockerPlatform | None:
Expand Down
2 changes: 1 addition & 1 deletion localstack/services/logs/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ def moto_filter_log_events(
):
# moto currently raises an exception if filter_patterns is None, so we skip it
events = filter_log_events(
self, start_time=start_time, end_time=end_time, filter_pattern=None, *args, **kwargs
self, *args, start_time=start_time, end_time=end_time, filter_pattern=None, **kwargs
)

if not filter_pattern:
Expand Down
2 changes: 1 addition & 1 deletion localstack/services/opensearch/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def init_directories(dirs: Directories):
chmod_r(dirs.backup, 0o777)

# clear potentially existing lock files (which cause problems since ES 7.10)
for d, dirs, files in os.walk(dirs.data, True):
for d, _, files in os.walk(dirs.data, True):
for f in files:
if f.endswith(".lock"):
rm_rf(os.path.join(d, f))
Expand Down
2 changes: 1 addition & 1 deletion localstack/services/s3/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def validate_website_configuration(website_config: WebsiteConfiguration) -> None
if len(routing_rules) == 0:
raise MalformedXML()
if len(routing_rules) > 50:
raise "Something?"
raise ValueError("Too many routing rules") # TODO: correct exception
for routing_rule in routing_rules:
redirect = routing_rule.get("Redirect", {})
# todo: this does not raise an error? check what GetWebsiteConfig returns? empty field?
Expand Down
7 changes: 2 additions & 5 deletions localstack/services/sns/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,13 @@ def publish_batch(
)

if is_fifo := (".fifo" in topic_arn):
if not all(["MessageGroupId" in entry for entry in publish_batch_request_entries]):
if not all("MessageGroupId" in entry for entry in publish_batch_request_entries):
raise InvalidParameterException(
"Invalid parameter: The MessageGroupId parameter is required for FIFO topics"
)
if moto_topic.content_based_deduplication == "false":
if not all(
[
"MessageDeduplicationId" in entry
for entry in publish_batch_request_entries
]
"MessageDeduplicationId" in entry for entry in publish_batch_request_entries
):
raise InvalidParameterException(
"Invalid parameter: The topic should either have ContentBasedDeduplication enabled or MessageDeduplicationId provided explicitly",
Expand Down
2 changes: 1 addition & 1 deletion localstack/services/ssm/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def get_parameters(
if SsmProvider._has_secrets(names):
return SsmProvider._get_params_and_secrets(context.account_id, context.region, names)

norm_names = list([SsmProvider._normalize_name(name, validate=True) for name in names])
norm_names = [SsmProvider._normalize_name(name, validate=True) for name in names]
request = {"Names": norm_names, "WithDecryption": bool(with_decryption)}
res = call_moto_with_request(context, request)

Expand Down
4 changes: 2 additions & 2 deletions localstack/testing/pytest/path_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ def pytest_collection_modifyitems(config, items):
return # No filtering if the list is empty => full test suite

# this is technically redundant since we can just add "tests/" instead as a line item. still prefer to be explicit here
if any([p == SENTINEL_ALL_TESTS for p in pathfilter_substrings]):
if any(p == SENTINEL_ALL_TESTS for p in pathfilter_substrings):
return # at least one change should lead to a full run

# technically doesn't even need to be checked since the loop below will take care of it
if all([p == SENTINEL_NO_TEST for p in pathfilter_substrings]):
if all(p == SENTINEL_NO_TEST for p in pathfilter_substrings):
items[:] = []
# we only got sentinal values that signal a change that doesn't need to be tested, so delesect all
config.hook.pytest_deselected(items=items)
Expand Down
8 changes: 5 additions & 3 deletions localstack/testing/scenario/cdk_lambda_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def load_python_lambda_to_s3(
bucket_name: str,
key_name: str,
code_path: str,
additional_python_packages: list[str] = [],
additional_python_packages: list[str] = None,
):
"""
Helper function to setup Lambdas that need additional python libs.
Expand Down Expand Up @@ -65,8 +65,8 @@ def load_nodejs_lambda_to_s3(
bucket_name: str,
key_name: str,
code_path: str,
additional_nodjs_packages: list[str] = [],
additional_resources: list[str] = [],
additional_nodjs_packages: list[str] = None,
additional_resources: list[str] = None,
):
"""
Helper function to setup nodeJS Lambdas that need additional libs.
Expand All @@ -81,6 +81,8 @@ def load_nodejs_lambda_to_s3(
:param additional_resources: list of path-strings to resources or internal libs that should be packaged into the lambda
:return: None
"""
additional_resources = additional_resources or []

try:
temp_dir = tempfile.mkdtemp()
tmp_zip_path = os.path.join(tempfile.gettempdir(), "helper.zip")
Expand Down
1 change: 1 addition & 0 deletions localstack/testing/scenario/provisioning.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ def add_custom_setup_provisioning_step(self, setup_task: Callable):
warnings.warn(
"`add_custom_setup_provisioning_step` is deprecated. Use `add_custom_setup`",
DeprecationWarning,
stacklevel=2,
)
self.add_custom_setup(setup_task)

Expand Down
4 changes: 2 additions & 2 deletions localstack/utils/aws/templating.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ def render_vtl(self, template: str, variables: Dict, as_json=False) -> str | dic
return template

# fix "#set" commands
template = re.sub(r"(^|\n)#\s+set(.*)", r"\1#set\2", template, re.MULTILINE)
template = re.sub(r"(^|\n)#\s+set(.*)", r"\1#set\2", template, count=re.MULTILINE)

# enable syntax like "test#${foo.bar}"
empty_placeholder = " __pLaCe-HoLdEr__ "
template = re.sub(
r"([^\s]+)#\$({)?(.*)",
r"\1#%s$\2\3" % empty_placeholder,
template,
re.MULTILINE,
count=re.MULTILINE,
)

# add extensions for common string functions below
Expand Down
8 changes: 4 additions & 4 deletions localstack/utils/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,9 @@ def validate_localstack_config(name: str):
# prepare config options
container_name = ls_service_details.get("container_name") or ""
docker_ports = (port.split(":")[-2] for port in ls_service_details.get("ports", []))
docker_env = dict(
(env.split("=")[0], env.split("=")[1]) for env in ls_service_details.get("environment", {})
)
docker_env = {
env.split("=")[0]: env.split("=")[1] for env in ls_service_details.get("environment", {})
}
edge_port = config.GATEWAY_LISTEN[0].port
main_container = config.MAIN_CONTAINER_NAME

Expand Down Expand Up @@ -940,7 +940,7 @@ def attach(self):

def exec_in_container(self, *args, **kwargs):
return self.container_client.exec_in_container(
container_name_or_id=self.id, *args, **kwargs
*args, container_name_or_id=self.id, **kwargs
)

def stopped(self) -> Container:
Expand Down
2 changes: 1 addition & 1 deletion localstack/utils/container_utils/container_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ def list_containers(self, filter: Union[List[str], str, None] = None, all=True)
def get_running_container_names(self) -> List[str]:
"""Returns a list of the names of all running containers"""
result = self.list_containers(all=False)
result = list(map(lambda container: container["name"], result))
result = [container["name"] for container in result]
return result

def is_container_running(self, container_name: str) -> bool:
Expand Down
4 changes: 2 additions & 2 deletions localstack/utils/diagnose.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ def traverse_file_tree(root: str) -> List[str]:
try:
result = []
if config.in_docker():
for root, _, _ in os.walk(root):
result.append(root)
for dirpath, _, _ in os.walk(root):
result.append(dirpath)
return result
except Exception as e:
return ["traversing files failed %s" % e]
Expand Down
20 changes: 0 additions & 20 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -185,40 +185,20 @@ exclude = [

[tool.ruff.lint]
ignore = [
"B005", # TODO Using `.strip()` with multi-character strings is misleading
"B006", # TODO Do not use mutable data structures for argument defaults
"B007", # TODO Loop control variable x not used within loop body
"B008", # TODO Do not perform function call `Queue` in argument defaults
"B011", # TODO Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
"B016", # TODO Cannot raise a literal. Did you intend to return it or raise an Exception?
"B017", # TODO `pytest.raises(Exception)` should be considered evil
"B018", # TODO Found useless expression. Either assign it to a variable or remove it.
"B019", # TODO Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks
"B020", # TODO Loop control variable `invalid_values` overrides iterable it iterates
"B022", # TODO No arguments passed to `contextlib.suppress`. No exceptions will be suppressed and therefore this context manager is redundant
"B023", # TODO Function definition does not bind loop variable `server`
"B024", # TODO x is an abstract base class, but it has no abstract methods
"B026", # TODO Star-arg unpacking after a keyword argument is strongly discouraged
"B027", # TODO `Server.do_shutdown` is an empty method in an abstract base class, but has no abstract decorator
"B028", # TODO No explicit `stacklevel` keyword argument found
"B034", # TODO `re.sub` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
"B904", # TODO Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
"C401", # TODO Unnecessary generator (rewrite as a `set` comprehension)
"C402", # TODO Unnecessary generator (rewrite as a `dict` comprehension)
"C403", # TODO Unnecessary `list` comprehension (rewrite as a `set` comprehension)
"C405", # TODO Unnecessary `list` literal (rewrite as a `set` literal)
"C408", # TODO Unnecessary `list` call (rewrite as a literal)
"C411", # TODO Unnecessary `list` call (remove the outer call to `list()`)
"C414", # TODO Unnecessary `list` call within `sorted()`
"C416", # TODO Unnecessary `set` comprehension
"C417", # TODO Replace `map` with a generator expression
"C418", # TODO Unnecessary `dict` literal passed to `dict()` (remove the outer call to `dict()`)
"C419", # TODO Unnecessary list comprehension.
"C901", # TODO function is too complex
"E402", # TODO Module level import not at top of file
"E501", # E501 Line too long - handled by black, see https://docs.astral.sh/ruff/faq/#is-ruff-compatible-with-black
"E721", # TODO Do not compare types, use `isinstance()`
"F403", # TODO `from localstack.services.cloudformation.models import *` used; unable to detect undefined names
"T201", # TODO `print` found
"T203", # TODO `pprint` found
]
Expand Down
2 changes: 1 addition & 1 deletion tests/aws/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def snapshot(request, _snapshot_session: SnapshotSession, account_id, region_nam
"tests/aws/scenario/lambda_destination",
"tests/aws/scenario/loan_broker",
]
if any([e in request.fspath.dirname for e in exemptions]):
if any(e in request.fspath.dirname for e in exemptions):
_snapshot_session.add_transformer(SNAPSHOT_BASIC_TRANSFORMER, priority=2)
else:
_snapshot_session.add_transformer(SNAPSHOT_BASIC_TRANSFORMER_NEW, priority=2)
Expand Down
6 changes: 3 additions & 3 deletions tests/aws/services/cloudformation/api/test_changesets.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_create_change_set_without_parameters(
try:
# make sure the change set wasn't executed (which would create a topic)
topics = aws_client.sns.list_topics()
topic_arns = list(map(lambda x: x["TopicArn"], topics["Topics"]))
topic_arns = [x["TopicArn"] for x in topics["Topics"]]
assert not any("sns-topic-simple" in arn for arn in topic_arns)
# stack is initially in REVIEW_IN_PROGRESS state. only after executing the change_set will it change its status
stack_response = aws_client.cloudformation.describe_stacks(StackName=stack_id)
Expand Down Expand Up @@ -328,7 +328,7 @@ def test_create_change_set_with_ssm_parameter(
wait_until(is_stack_created(stack_id))

topics = aws_client.sns.list_topics()
topic_arns = list(map(lambda x: x["TopicArn"], topics["Topics"]))
topic_arns = [x["TopicArn"] for x in topics["Topics"]]
assert any((parameter_value in t) for t in topic_arns)
finally:
cleanup_changesets([change_set_id])
Expand Down Expand Up @@ -383,7 +383,7 @@ def test_execute_change_set(
assert wait_until(is_change_set_finished(change_set_id))
# check if stack resource was created
topics = aws_client.sns.list_topics()
topic_arns = list(map(lambda x: x["TopicArn"], topics["Topics"]))
topic_arns = [x["TopicArn"] for x in topics["Topics"]]
assert any(("sns-topic-simple" in t) for t in topic_arns)

# new change set name
Expand Down
6 changes: 3 additions & 3 deletions tests/aws/services/cloudformation/api/test_stacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def test_list_stack_resources_for_removed_resource(self, deploy_cfn_template, aw
]
resources_before = len(resources)
assert resources_before == 3
statuses = set([res["ResourceStatus"] for res in resources])
statuses = {res["ResourceStatus"] for res in resources}
assert statuses == {"CREATE_COMPLETE"}

# remove one resource from the template, then update stack (via change set)
Expand All @@ -218,7 +218,7 @@ def test_list_stack_resources_for_removed_resource(self, deploy_cfn_template, aw
"StackResourceSummaries"
]
assert len(resources) == resources_before - 1
statuses = set([res["ResourceStatus"] for res in resources])
statuses = {res["ResourceStatus"] for res in resources}
assert statuses == {"UPDATE_COMPLETE"}

@markers.aws.needs_fixing
Expand Down Expand Up @@ -579,7 +579,7 @@ def test_events_resource_types(deploy_cfn_template, snapshot, aws_client):
"StackEvents"
]

resource_types = list(set([event["ResourceType"] for event in events]))
resource_types = list({event["ResourceType"] for event in events})
resource_types.sort()
snapshot.match("resource_types", resource_types)

Expand Down

0 comments on commit 628ed08

Please sign in to comment.