Skip to content

MPT-11914 Initial setup of wps and ruff rules#477

Merged
d3rky merged 1 commit intomainfrom
feature/MPT-11914/adjust-ruff-rules
Aug 5, 2025
Merged

MPT-11914 Initial setup of wps and ruff rules#477
d3rky merged 1 commit intomainfrom
feature/MPT-11914/adjust-ruff-rules

Conversation

@d3rky
Copy link
Copy Markdown
Collaborator

@d3rky d3rky commented Jul 26, 2025

No description provided.

Comment thread uv.lock Outdated
@d3rky d3rky force-pushed the feature/MPT-11914/adjust-ruff-rules branch 5 times, most recently from d1d097e to b6d243d Compare August 4, 2025 18:49
@softwareone-platform softwareone-platform deleted a comment from sonarqubecloud Bot Aug 4, 2025
@d3rky d3rky marked this pull request as ready for review August 4, 2025 18:50
@softwareone-platform softwareone-platform deleted a comment from sonarqubecloud Bot Aug 4, 2025
@d3rky d3rky force-pushed the feature/MPT-11914/adjust-ruff-rules branch from b6d243d to 8063a44 Compare August 4, 2025 18:52


class Command(BaseCommand):
class Command(AdobeBaseCommand):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@jentyk @alephsur what is the difference between this command and process_3yc?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This command covers the requirement from Sync Improvements. It syncs only 3yc_enroll status if it is in one of the temporary states and does full sync when it changes to commited. process_3yc does some other stuff as well which I do not fully understand, but it is definitely something more than just a synchronization. I think it make sense to keep pure sync separately from other operations.

Comment thread adobe_vipm/apps.py
Comment thread adobe_vipm/flows/pipeline.py
)


def resubmit_3yc_commitment_request(mpt_client, is_recommitment=False):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@jentyk @alephsur why this code is not used anymore? Extension now doesn't support recommitment requests? why?

Is it some new requirement from Stu?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure, but it wasn't used yet when I joined

Comment thread adobe_vipm/adobe/client.py Outdated
Comment thread adobe_vipm/adobe/config.py Outdated
Comment thread adobe_vipm/adobe/config.py Outdated
Comment thread adobe_vipm/adobe/config.py
Comment thread adobe_vipm/adobe/config.py
Comment thread adobe_vipm/adobe/dataclasses.py
Comment thread adobe_vipm/adobe/errors.py
Comment thread adobe_vipm/adobe/errors.py Outdated
@d3rky d3rky force-pushed the feature/MPT-11914/adjust-ruff-rules branch from 8063a44 to ca85fc9 Compare August 5, 2025 08:43
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Aug 5, 2025

Comment on lines +21 to +23
"""

Create Reseller Account on Adobe.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""
Create Reseller Account on Adobe.
"""Create Reseller Account on Adobe.

response.raise_for_status()
return response.json()

def _do_not_make_return_params(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's strange that mypy doesn't report anything about the typing. Do we have the correct config?

Suggested change
def _do_not_make_return_params(self):
def _do_not_make_return_params(self) -> dict[str, str]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah! I've just realized we don't have mypy in this repo. 😢

Comment thread adobe_vipm/adobe/utils.py
Comment on lines +13 to 15
line_items (list): List of item to search.
sku (str): The partial SKU to search in
the list of item.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
line_items (list): List of item to search.
sku (str): The partial SKU to search in
the list of item.
line_items: List of item to search.
sku: The partial SKU to search in the list of item.

Comment thread adobe_vipm/adobe/utils.py


def get_item_by_partial_sku(items, sku):
def get_item_by_partial_sku(line_items, sku):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
def get_item_by_partial_sku(line_items, sku):
def get_item_by_partial_sku(line_items: list, sku: str) -> str:

Comment thread adobe_vipm/adobe/utils.py
Comment on lines +29 to 32
Converts Marketplace Line id to integer by extracting sequencial part of the line id.

Example: ALI-1234-1234-1234-0001 --> 1
"""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed the Args and return sections

Comment on lines +328 to 329
def get_transfers_to_check(product_id: str):
"""
Copy link
Copy Markdown
Contributor

@svazquezco svazquezco Aug 5, 2025

Choose a reason for hiding this comment

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

Looks like we also forgot the return typing in the methods below. They're not required now but...just in case

Suggested change
def get_transfers_to_check(product_id: str):
"""
def get_transfers_to_check(product_id: str) -> list:
"""

from django.core.management.base import BaseCommand


class AdobeBaseCommand(BaseCommand):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❤️

Comment thread pyproject.toml
Comment on lines +172 to 190
# Plugin configs:
flake8-import-conventions.banned-from = [ "ast", "datetime" ]
flake8-import-conventions.aliases = { datetime = "dt" }
flake8-quotes.inline-quotes = "double"
mccabe.max-complexity = 6
pydocstyle.convention = "google"

[tool.ruff.lint.per-file-ignores]
"tests/*.py" = [
"D103", # missing docstring in public function
"S101", # asserts
"S105", # hardcoded passwords
"S404", # subprocess calls are for tests
"S603", # do not require `shell=True`
"S607", # partial executable paths
]

[tool.ruff.lint.isort]
known-third-party = ["swo"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to use the new ruff format

Suggested change
# Plugin configs:
flake8-import-conventions.banned-from = [ "ast", "datetime" ]
flake8-import-conventions.aliases = { datetime = "dt" }
flake8-quotes.inline-quotes = "double"
mccabe.max-complexity = 6
pydocstyle.convention = "google"
[tool.ruff.lint.per-file-ignores]
"tests/*.py" = [
"D103", # missing docstring in public function
"S101", # asserts
"S105", # hardcoded passwords
"S404", # subprocess calls are for tests
"S603", # do not require `shell=True`
"S607", # partial executable paths
]
[tool.ruff.lint.isort]
known-third-party = ["swo"]
[tool.ruff.lint.flake8-import-conventions]
aliases = { datetime = "dt" }
banned-from = [ "ast", "datetime" ]
[tool.ruff.lint.flake8-quotes]
inline-quotes = "double"
[tool.ruff.lint.isort]
known-third-party = ["swo"]
[tool.ruff.lint.mccabe]
max-complexity = 6
[tool.ruff.lint.per-file-ignores]
"tests/*.py" = [
"D103", # missing docstring in public function
"S101", # asserts
"S105", # hardcoded passwords
"S404", # subprocess calls are for tests
"S603", # do not require `shell=True`
"S607", # partial executable paths
]
[tool.ruff.lint.pydocstyle]
convention = "google"

Copy link
Copy Markdown
Contributor

@svazquezco svazquezco left a comment

Choose a reason for hiding this comment

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

Great refactor! ❤️ That was a lot of work! I'm sure this will help us reach the next level 💪

@d3rky d3rky merged commit 3c57180 into main Aug 5, 2025
3 checks passed
@d3rky d3rky deleted the feature/MPT-11914/adjust-ruff-rules branch August 5, 2025 10:05
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.

5 participants