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 unnecessary-dunder-call #30365

Merged
merged 10 commits into from
Dec 11, 2024
Merged

{Pylint} Fix unnecessary-dunder-call #30365

merged 10 commits into from
Dec 11, 2024

Conversation

bebound
Copy link
Contributor

@bebound bebound commented Nov 15, 2024

Fix this with ruff check . --select PLC2801 --fix --preview --unsafe-fixes

unnecessary-dunder-call was disabled in #26685

Ref:
https://pylint.readthedocs.io/en/stable/user_guide/messages/convention/unnecessary-dunder-call.html
https://docs.astral.sh/ruff/rules/unnecessary-dunder-call/

Copy link

azure-client-tools-bot-prd bot commented Nov 15, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.12
️✔️3.9
️✔️computefleet
️✔️latest
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.12
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.12
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.12
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.12
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.12
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9

Copy link

azure-client-tools-bot-prd bot commented Nov 15, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Nov 15, 2024

Pylint

@@ -229,7 +229,7 @@ def test_load_cert_file(self):
try:
blob, thumbprint = load_cert_file(pfx_file, testpassword)
except CLIInternalError as e:
self.assertTrue(e.error_msg.error_msg.__contains__('Invalid password or PKCS12 data'))
self.assertTrue(e.error_msg.error_msg in 'Invalid password or PKCS12 data')
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use assertIn

@@ -63,7 +63,7 @@ def _validate_deployment_name_with_template_specs(namespace):
namespace.template_spec = namespace.template_spec.strip("\"")
if not is_valid_resource_id(namespace.template_spec):
raise CLIError('--template-spec is not a valid resource ID.')
if namespace.template_spec.__contains__("versions") is False:
if (namespace.template_spec in "versions") is False:
Copy link
Contributor

Choose a reason for hiding this comment

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

if not ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check all code later. Some codes are so werid...

Copy link
Contributor Author

@bebound bebound Nov 18, 2024

Choose a reason for hiding this comment

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

These variables are also reversed, it should be "versions" in namespace.template_spec.

Reported to astral-sh/ruff#14423

@@ -481,23 +481,23 @@ def test_botservice_create_should_raise_error_for_invalid_app_id_args(self, reso
self.cmd('az bot create -g {rg} -n {botname} --appid {numbers_id} --app-type MultiTenant')
raise AssertionError()
except CLIError as cli_error:
assert cli_error.__str__() == expected_error
assert (str(cli_error)) == expected_error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are nnnecessary parentheses.

Reported to astral-sh/ruff#14597

@@ -316,7 +316,7 @@ def flush():
save(get_config_dir(), _session.generate_payload())

# reset session fields, retaining correlation id and application
_session.__init__(correlation_id=_session.correlation_id, application=_session.application)
_session.__init__(correlation_id=_session.correlation_id, application=_session.application) # pylint: disable=unnecessary-dunder-call
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses __init__ to reset fields, so weird.

Copy link
Member

Choose a reason for hiding this comment

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

@evelyn-ys for awareness.

Copy link
Member

@evelyn-ys evelyn-ys Dec 11, 2024

Choose a reason for hiding this comment

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

I don't know why the original author write in this way. Maybe we can try replace with

_session = TelemetrySession(correlation_id=_session.correlation_id, application=_session.application)

@bebound
Copy link
Contributor Author

bebound commented Nov 28, 2024

This function contains so many Python black magic, it seems it not used anymore.
@necusjz for confirmation

def delete_lb_resource_property_entry(resource, prop):
""" Factory method for creating delete functions. """
def delete_func(cmd, resource_group_name, resource_name, item_name, no_wait=False): # pylint: disable=unused-argument
client = getattr(network_client_factory(cmd.cli_ctx), resource)
item = lb_get(client, resource_group_name, resource_name)
if item.__getattribute__(prop) is not None:
keep_items = [x for x in item.__getattribute__(prop) if x.name.lower() != item_name.lower()]
else:
keep_items = None
with cmd.update_context(item) as c:
c.set_param(prop, keep_items)
if no_wait:
sdk_no_wait(no_wait, client.begin_create_or_update, resource_group_name, resource_name, item)
else:
result = sdk_no_wait(no_wait, client.begin_create_or_update,
resource_group_name, resource_name, item).result()
if next((x for x in getattr(result, prop) or [] if x.name.lower() == item_name.lower()), None):
raise CLIError("Failed to delete '{}' on '{}'".format(item_name, resource_name))
func_name = 'delete_lb_resource_property_entry_{}_{}'.format(resource, prop)
setattr(sys.modules[__name__], func_name, delete_func)
return func_name

setattr(sys.modules[__name__], func_name, delete_func)
return func_name


Copy link
Contributor Author

@bebound bebound Nov 28, 2024

Choose a reason for hiding this comment

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

Copy link
Member

@jiasli jiasli Nov 29, 2024

Choose a reason for hiding this comment

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

I would suggest moving this change to a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move to #30453

@wangzelin007
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bebound bebound marked this pull request as ready for review November 29, 2024 05:40
@jiasli
Copy link
Member

jiasli commented Nov 29, 2024

in takes higher precedence over or/and: https://docs.python.org/3/reference/expressions.html#operator-precedence, so there is no need to add parentheses for in operations.

wangzelin007
wangzelin007 previously approved these changes Dec 3, 2024
@bebound bebound merged commit d9de029 into Azure:dev Dec 11, 2024
53 checks passed
@bebound bebound deleted the fix-dunder branch December 11, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants