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

{Pylint} Fix consider-using-in #30349

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft
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
1 change: 0 additions & 1 deletion pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ disable=
wrong-import-order,
consider-using-f-string,
unspecified-encoding,
consider-using-in,
# These rules were added in Pylint >= 2.12, disable them to avoid making retroactive change
missing-timeout,
# These rules were added in Pylint >= 3.2
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli-core/azure/cli/core/aaz/_arg.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __getitem__(self, data):
if isinstance(self.items, dict):
# convert data, it can be key, value or key.lower() when not case sensitive
for k, v in self.items.items():
if v == data or k == data or not self._case_sensitive and k.lower() == data.lower():
if data in (v, k) or not self._case_sensitive and k.lower() == data.lower():
key = k
break

Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli-core/azure/cli/core/aaz/_operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def serialize_url_param(name, value, required=True, skip_quote=False, **kwargs):
if isinstance(value, AAZBaseValue):
value = value.to_serialized_data()

if value == AAZUndefined or value == None: # noqa: E711, pylint: disable=singleton-comparison
if value in (AAZUndefined, None):
if required:
raise ValueError(f"url parameter {name} is required.")
return {} # return empty dict
Expand Down Expand Up @@ -199,7 +199,7 @@ def processor(schema, result):
else:
data = value

if data == AAZUndefined or data == None: # noqa: E711, pylint: disable=singleton-comparison
if data in (AAZUndefined, None):
if required:
raise ValueError("Missing request content")
return None
Expand Down
6 changes: 3 additions & 3 deletions src/azure-cli-core/azure/cli/core/extension/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def _validate_whl_extension(ext_file):

def _get_extension_info_from_source(source):
url_parse_result = urlparse(source)
is_url = (url_parse_result.scheme == 'http' or url_parse_result.scheme == 'https')
is_url = (url_parse_result.scheme in ('http', 'https'))
whl_filename = os.path.basename(url_parse_result.path) if is_url else os.path.basename(source)
parsed_filename = WHEEL_INFO_RE(whl_filename)
# Extension names can have - but .whl format changes it to _ (PEP 0427). Undo this.
Expand All @@ -100,7 +100,7 @@ def _add_whl_ext(cli_ctx, source, ext_sha256=None, pip_extra_index_urls=None, pi
if not source.endswith('.whl'):
raise ValueError('Unknown extension type. Only Python wheels are supported.')
url_parse_result = urlparse(source)
is_url = (url_parse_result.scheme == 'http' or url_parse_result.scheme == 'https')
is_url = (url_parse_result.scheme in ('http', 'https'))
logger.debug('Extension source is url? %s', is_url)
whl_filename = os.path.basename(url_parse_result.path) if is_url else os.path.basename(source)
parsed_filename = WHEEL_INFO_RE(whl_filename)
Expand All @@ -110,7 +110,7 @@ def _add_whl_ext(cli_ctx, source, ext_sha256=None, pip_extra_index_urls=None, pi
raise CLIError('Unable to determine extension name from {}. Is the file name correct?'.format(source))
if extension_exists(extension_name, ext_type=WheelExtension):
raise CLIError('The extension {} already exists.'.format(extension_name))
if extension_name == 'rdbms-connect' or extension_name == 'serviceconnector-passwordless':
if extension_name in ('rdbms-connect', 'serviceconnector-passwordless'):
_install_deps_for_psycopg2()
ext_file = None
if is_url:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def resource(self):

def _set_operation_status(self, response):
AgentPoolStatus = self._cmd.get_models('ProvisioningState')
if response.http_response.status_code == 200 or response.http_response.status_code == 404:
if response.http_response.status_code in (200, 404):
self.operation_result = self._deserialize(response)
self.operation_status = self.operation_result.provisioning_state or AgentPoolStatus.succeeded.value
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def request_data_from_registry(http_method,
result = response.json()[result_index] if result_index else response.json()
next_link = response.headers['link'] if 'link' in response.headers else None
return result, next_link, response.status_code
if response.status_code == 201 or response.status_code == 202:
if response.status_code in (201, 202):
result = None
try:
result = response.json()[result_index] if result_index else response.json()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def _get_last_streaming_operation(cmd,


def _is_ongoing_streaming_status(status):
res = status == ArtifactStreamingStatus.Running.value or status == ArtifactStreamingStatus.NotStarted.value
res = status in (ArtifactStreamingStatus.Running.value, ArtifactStreamingStatus.NotStarted.value)
return res


Expand Down
6 changes: 2 additions & 4 deletions src/azure-cli/azure/cli/command_modules/acs/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,7 @@ def validate_priority(namespace):
if namespace.priority is not None:
if namespace.priority == '':
return
if namespace.priority != "Spot" and \
namespace.priority != "Regular":
if namespace.priority not in ('Spot', 'Regular'):
raise CLIError("--priority can only be Spot or Regular")


Expand All @@ -320,8 +319,7 @@ def validate_eviction_policy(namespace):
if namespace.eviction_policy is not None:
if namespace.eviction_policy == '':
return
if namespace.eviction_policy != "Delete" and \
namespace.eviction_policy != "Deallocate":
if namespace.eviction_policy not in ('Delete', 'Deallocate'):
raise CLIError("--eviction-policy can only be Delete or Deallocate")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ def _get_os_type(self, read_only: bool = False) -> Union[str, None]:
raw_os_sku = self.raw_param.get("os_sku")
sku_2019 = CONST_OS_SKU_WINDOWS2019
sku_2022 = CONST_OS_SKU_WINDOWS2022
if raw_os_sku == sku_2019 or raw_os_sku == sku_2022:
if raw_os_sku in (sku_2019, sku_2022):
raise InvalidArgumentValueError(
"OS SKU is invalid for Linux OS Type."
" Please specify '--os-type Windows' for Windows SKUs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ def validate_disable_azure_container_storage_params( # pylint: disable=too-many
number_of_storagepool_types_active += 1

number_of_storagepool_types_to_be_disabled = 0
if storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK or \
storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN or \
if storage_pool_type in (CONST_STORAGE_POOL_TYPE_AZURE_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN) or \
(storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and
storage_pool_option != CONST_ACSTOR_ALL):
number_of_storagepool_types_to_be_disabled = 1
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/azure/cli/command_modules/acs/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ def aks_upgrade(cmd,
disable_force_upgrade=disable_force_upgrade,
upgrade_override_until=upgrade_override_until)

if instance.kubernetes_version == kubernetes_version or kubernetes_version == '':
if kubernetes_version in (instance.kubernetes_version, ''):
# don't prompt here because there is another prompt below?
if instance.provisioning_state == "Succeeded":
logger.warning("The cluster is already on version %s and is not in a failed state. No operations "
Expand Down Expand Up @@ -2517,7 +2517,7 @@ def aks_agentpool_upgrade(cmd, client, resource_group_name, cluster_name,

instance = client.get(resource_group_name, cluster_name, nodepool_name)

if kubernetes_version == '' or instance.orchestrator_version == kubernetes_version:
if kubernetes_version in ('', instance.orchestrator_version):
msg = "The new kubernetes version is the same as the current kubernetes version."
if instance.provisioning_state == "Succeeded":
msg = "The cluster is already on version {} and is not in a failed state. No operations will occur when upgrading to the same version if the cluster is not in a failed state.".format(instance.orchestrator_version)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def getMaintenanceConfiguration(cmd, raw_parameters):
config_name = raw_parameters.get("config_name")
if config_name == CONST_DEFAULT_CONFIGURATION_NAME:
return constructDefaultMaintenanceConfiguration(cmd, raw_parameters)
if config_name == CONST_AUTOUPGRADE_CONFIGURATION_NAME or config_name == CONST_NODEOSUPGRADE_CONFIGURATION_NAME:
if config_name in (CONST_AUTOUPGRADE_CONFIGURATION_NAME, CONST_NODEOSUPGRADE_CONFIGURATION_NAME):
return constructDedicatedMaintenanceConfiguration(cmd, raw_parameters)

raise InvalidArgumentValueError('--config-name must be one of default, aksManagedAutoUpgradeSchedule or aksManagedNodeOSUpgradeSchedule, not {}'.format(config_name))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2202,10 +2202,7 @@ def _get_outbound_type(

# dynamic completion
if (
not read_from_mc and
outbound_type != CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY and
outbound_type != CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY and
outbound_type != CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING
not read_from_mc and outbound_type not in (CONST_OUTBOUND_TYPE_MANAGED_NAT_GATEWAY, CONST_OUTBOUND_TYPE_USER_ASSIGNED_NAT_GATEWAY, CONST_OUTBOUND_TYPE_USER_DEFINED_ROUTING)
):
outbound_type = CONST_OUTBOUND_TYPE_LOAD_BALANCER

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def validate_arguments(preset, insights_to_extract, audio_language, resolution):
if insights_to_extract and preset != 'VideoAnalyzer':
raise CLIError("insights-to-extract argument only works with VideoAnalyzer preset type.")

if audio_language and preset != 'VideoAnalyzer' and preset != 'AudioAnalyzer':
if audio_language and preset not in ('VideoAnalyzer', 'AudioAnalyzer'):
raise CLIError("audio-language argument only works with VideoAnalyzer or AudioAnalyzer preset types.")

if resolution and preset != 'FaceDetector':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def validate_import_key(key):
if not isinstance(key, str):
logger.warning("Ignoring invalid key '%s'. Key must be a string.", key)
return False
if key == '.' or key == '..' or '%' in key:
if key in (".", "..") or "%" in key:
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 why ruff changes ' to ".

Copy link
Contributor

Choose a reason for hiding this comment

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

Black?

Copy link
Member Author

Choose a reason for hiding this comment

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

@atombrella, could you explain in detail?

Copy link
Contributor

@atombrella atombrella Dec 12, 2024

Choose a reason for hiding this comment

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

My guess is that Ruff would format according to Black. In the documentation for Ruff, it's described as a "drop-in replacement for Black". https://docs.astral.sh/ruff/formatter/ But I don't know for sure, because it doesn't seem to change the quotes consistently in this change.

https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#strings

Copy link
Contributor

@bebound bebound Dec 16, 2024

Choose a reason for hiding this comment

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

@jiasli, you can create an issue for ruff, they are very responsive.
In https://github.com/Azure/azure-cli/pull/30349/files#diff-e75fafa4284121a8bbc79ffdf1ea7241768bedd4b96f16f61adaee60c0f7e679R67, it changes " to '

I think keep the original character is the expected behavior.

PS: I like ' because it saves a <shift> keystroke compared to ".

logger.warning("Ignoring invalid key '%s'. Key cannot be a '.' or '..', or contain the '%%' character.", key)
return False
if key.startswith(FeatureFlagConstants.FEATURE_FLAG_PREFIX):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def validate_key(namespace):
raise RequiredArgumentMissingError("Key cannot be empty.")

input_key = str(namespace.key).lower()
if input_key == '.' or input_key == '..' or '%' in input_key:
if input_key in ('.', '..') or '%' in input_key:
raise InvalidArgumentValueError("Key is invalid. Key cannot be a '.' or '..', or contain the '%' character.")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ def _validate_ase_is_v3(ase):


def _validate_ase_not_ilb(ase):
if (ase.internal_load_balancing_mode != 0) and (ase.internal_load_balancing_mode != "None"):
Copy link
Member Author

Choose a reason for hiding this comment

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

() are unnecessary as != has higher precedence: https://docs.python.org/3/reference/expressions.html#operator-precedence

if ase.internal_load_balancing_mode not in (0, "None"):
raise ValidationError("Internal Load Balancer (ILB) App Service Environments not supported")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1323,8 +1323,7 @@ def setter(webapp):
webapp.identity.user_assigned_identities = None
if remove_local_identity:
webapp.identity.type = (IdentityType.none
if webapp.identity.type == IdentityType.system_assigned or
webapp.identity.type == IdentityType.none
if webapp.identity.type in (IdentityType.system_assigned, IdentityType.none)
else IdentityType.user_assigned)

if webapp.identity.type not in [IdentityType.none, IdentityType.system_assigned]:
Expand Down Expand Up @@ -7119,7 +7118,7 @@ def _make_onedeploy_request(params):

# check the status of deployment
# pylint: disable=too-many-nested-blocks
if response.status_code == 202 or response.status_code == 200:
if response.status_code in (202, 200):
response_body = None
if poll_async_deployment_for_debugging:
if params.track_status is not None and params.track_status:
Expand Down Expand Up @@ -7290,8 +7289,7 @@ def _verify_hostname_binding(cmd, resource_group_name, name, hostname, slot=None
verified_hostname_found = False
for hostname_binding in hostname_bindings:
binding_name = hostname_binding.name.split('/')[-1]
if binding_name.lower() == hostname and (hostname_binding.host_name_type == 'Verified' or
hostname_binding.host_name_type == 'Managed'):
if binding_name.lower() == hostname and (hostname_binding.host_name_type in ('Verified', 'Managed')):
verified_hostname_found = True

return verified_hostname_found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,7 @@ def setter(staticsite):
staticsite.identity.user_assigned_identities = None
if remove_local_identity:
staticsite.identity.type = (IdentityType.none
if staticsite.identity.type == IdentityType.system_assigned or
staticsite.identity.type == IdentityType.none
if staticsite.identity.type in (IdentityType.system_assigned, IdentityType.none)
else IdentityType.user_assigned)

if staticsite.identity.type not in [IdentityType.none, IdentityType.system_assigned]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def pre_operations(self):
user_assigned_identities = {}
for identity in args.user_assigned_identities:
user_assigned_identities[identity.to_serialized_data()] = {}
if args.identity_type == 'UserAssigned' or args.identity_type == 'SystemAssigned, UserAssigned':
if args.identity_type in ('UserAssigned', 'SystemAssigned, UserAssigned'):
args.identity = {
'type': args.identity_type,
'userAssignedIdentities': user_assigned_identities
Expand Down Expand Up @@ -200,7 +200,7 @@ def pre_operations(self):
user_assigned_identities = {}
for identity in args.user_assigned_identities:
user_assigned_identities[identity.to_serialized_data()] = {}
if args.identity_type == 'UserAssigned' or args.identity_type == 'SystemAssigned, UserAssigned':
if args.identity_type in ('UserAssigned', 'SystemAssigned, UserAssigned'):
args.identity = {
'type': args.identity_type,
'userAssignedIdentities': user_assigned_identities
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,7 @@ def post_operations(self):
user_assigned_identities = {}
for identity in args.user_assigned_identities:
user_assigned_identities[identity.to_serialized_data()] = {}
if args.identity_type == 'UserAssigned' or args.identity_type == 'SystemAssigned, UserAssigned':
if args.identity_type in ("UserAssigned", "SystemAssigned, UserAssigned"):
identity = {
'type': args.identity_type,
'userAssignedIdentities': user_assigned_identities
Expand Down
6 changes: 3 additions & 3 deletions src/azure-cli/azure/cli/command_modules/container/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ def _get_subnet_id(cmd, location, resource_group_name, vnet, vnet_address_prefix
"resource_group": resource_group_name
})
except HttpResponseError as ex:
if ex.error.code == "NotFound" or ex.error.code == "ResourceNotFound":
if ex.error.code in ('NotFound', 'ResourceNotFound'):
subnet = None
else:
raise
Expand Down Expand Up @@ -580,7 +580,7 @@ def _get_subnet_id(cmd, location, resource_group_name, vnet, vnet_address_prefix
"resource_group": resource_group_name
})
except HttpResponseError as ex:
if ex.error.code == "NotFound" or ex.error.code == "ResourceNotFound":
if ex.error.code in ('NotFound', 'ResourceNotFound'):
vnet = None
else:
raise
Expand Down Expand Up @@ -1176,7 +1176,7 @@ def _is_container_terminated(client, resource_group_name, name, container_name):

# If a container group is terminated, assume the container is also terminated.
if container_group.instance_view and container_group.instance_view.state:
if container_group.instance_view.state == 'Succeeded' or container_group.instance_view.state == 'Failed':
if container_group.instance_view.state in ('Succeeded', 'Failed'):
return True

# If the restart policy is Always, assume the container will be restarted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ def generate_randomized_cert_name(thumbprint, prefix, initial="rg"):
from random import randint
cert_name = "{}-{}-{}-{:04}".format(prefix[:14], initial[:14], thumbprint[:4].lower(), randint(0, 9999))
for c in cert_name:
if not (c.isalnum() or c == '-' or c == '.'):
if not (c.isalnum() or c in ('-', '.')):
cert_name = cert_name.replace(c, '-')
return cert_name.lower()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ def _validate_included_paths_in_cep(partition_key_path, includedPaths, policyFor
raise InvalidArgumentValueError("Invalid encryptionType included in Client Encryption Policy. "
"encryptionType cannot be null or empty.")

if (encryptionType != "Deterministic" and encryptionType != "Randomized"):
if (encryptionType not in ('Deterministic', 'Randomized')):
raise InvalidArgumentValueError(f"Invalid Encryption Type {encryptionType} used. "
"Supported types are Deterministic or Randomized.")

Expand Down
6 changes: 3 additions & 3 deletions src/azure-cli/azure/cli/command_modules/cosmosdb/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ def cli_cosmosdb_identity_assign(client,
new_user_identities = [x for x in identities if x != SYSTEM_ID]

only_enabling_system = enable_system and len(new_user_identities) == 0
system_already_added = existing.identity.type == ResourceIdentityType.system_assigned or existing.identity.type == ResourceIdentityType.system_assigned_user_assigned
system_already_added = existing.identity.type in (ResourceIdentityType.system_assigned, ResourceIdentityType.system_assigned_user_assigned)
if only_enabling_system and system_already_added:
return existing.identity

Expand Down Expand Up @@ -2766,7 +2766,7 @@ def process_restorable_databases(restorable_databases, database_name):
if resource.operation_type == "Delete" and latest_database_delete_time < event_timestamp:
latest_database_delete_time = event_timestamp

if (resource.operation_type == "Create" or resource.operation_type == "Recreate") and latest_database_create_or_recreate_time < event_timestamp:
if (resource.operation_type in ("Create", "Recreate")) and latest_database_create_or_recreate_time < event_timestamp:
latest_database_create_or_recreate_time = event_timestamp

if database_rid is None:
Expand All @@ -2793,7 +2793,7 @@ def process_restorable_collections(restorable_collections, collection_name, data
if resource.operation_type == "Delete" and latest_collection_delete_time < event_timestamp:
latest_collection_delete_time = event_timestamp

if (resource.operation_type == "Create" or resource.operation_type == "Recreate") and latest_collection_create_or_recreate_time < event_timestamp:
if (resource.operation_type in ("Create", "Recreate")) and latest_collection_create_or_recreate_time < event_timestamp:
latest_collection_create_or_recreate_time = event_timestamp

if collection_rid is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def get_action(self, values, option_string): # pylint: disable=no-self-use
if k == 'id':
VirtualNetworkList["id"] = v
elif k == 'ignore-missing-endpoint':
if v == "true" or v == "True" or v == "TRUE":
if v in ('true', 'True', 'TRUE'):
VirtualNetworkList["ignore_missing_endpoint"] = True
else:
VirtualNetworkList["ignore_missing_endpoint"] = False
Expand Down
Loading
Loading