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

refactor(typing): reduce aws_lambda_powertools.shared.types usage #4896

Conversation

ericbn
Copy link
Contributor

@ericbn ericbn commented Aug 6, 2024

Issue number: #4897

Summary

Changes

Reduce aws_lambda_powertools.shared.types usage. The goal was to remove the package as a whole, but the custom AnyCallableT and JSONType defined there cannot be replaced by any type provided in typing or typing_extensions.

User experience

As discussed in #4607, this is an intermediary step towards adding from __future__ import annotations to all files. The refactoring in this PR will help ruff better help us refactor the code once we introduce the TCH and UP rules as discussed in the main issue.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

As discussed in aws-powertools#4607. This simplifies linting and refactoring so we can
introduce

    from __future__ import annotations

to all files, which is the plan as the next step.
@ericbn ericbn requested a review from a team as a code owner August 6, 2024 17:38
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 6, 2024
@leandrodamascena leandrodamascena linked an issue Aug 6, 2024 that may be closed by this pull request
2 tasks
@leandrodamascena
Copy link
Contributor

Reviewing now.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Aug 6, 2024
Copy link

sonarqubecloud bot commented Aug 6, 2024

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hey @ericbn I did some changes here.

  • I fixed the documentation, some highlights were wrong because we changed imports.
  • I removed JSONType from shared and moved it to Feature Flags, makes more sense.
  • I fixed absolute imports in Feature Flags - unrelated, but always good.

Can you please take a look before we approve and merge?

aws_lambda_powertools/shared/types.py Outdated Show resolved Hide resolved
@ericbn
Copy link
Contributor Author

ericbn commented Aug 6, 2024

Oi @leandrodamascena. All looking good to me. Good catch on the absolute imports and good to know about hl_lines which I totally missed.

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

APPROVED! @ericbn 🥇

@leandrodamascena
Copy link
Contributor

Oi @leandrodamascena.

Olá :D

@github-actions github-actions bot added enhancement and removed documentation Improvements or additions to documentation labels Aug 6, 2024
@leandrodamascena leandrodamascena merged commit 1a8818d into aws-powertools:v3 Aug 6, 2024
11 checks passed
@ericbn ericbn deleted the reduce-aws_lambda_powertools.shared.types-usage branch August 6, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commons enhancement event_handlers logger metrics size/L Denotes a PR that changes 100-499 lines, ignoring generated files. streaming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tech debt: Remove usages of aws_lambda_powertools.shared.types
2 participants