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

Redis: Add missing exported names #10151

Merged
merged 2 commits into from
May 6, 2023
Merged

Conversation

mjpieters
Copy link
Contributor

@mjpieters mjpieters commented May 6, 2023

  • redis.asyncio.RedisCluster / redis.asyncio.CommandsParser added in 4.3.0
  • redis.default_backoff / redis.asyncio.default_backoff, added in 4.4.0

- redis.asyncio.RedisCluster / redis.asyncio.CommandsParser, added in 4.3.0
- redis.default_backoff / redis.asyncio.default_backoff, added in 4.4.0
@AlexWaygood
Copy link
Member

(Nit: we prefer to avoid force pushes where possible at typeshed, as it makes it hard to see what changed between commits, and doesn't play very well with the GitHub UI. All PRs are squash-merged at typeshed, so there's no need to worry about a clean git history on the PR :)

@github-actions

This comment has been minimized.

@mjpieters
Copy link
Contributor Author

(Nit: we prefer to avoid force pushes where possible at typeshed, as it makes it hard to see what changed between commits, and doesn't play very well with the GitHub UI. All PRs are squash-merged at typeshed, so there's no need to worry about a clean git history on the PR :)

Ah, I'll keep that in mind!

Yes, I normally aim to provide a clean commit series, as a small mistake like forgetting to put default_backoff in the __all__ list is not something a reviewer would need to care about. 😄 So, my motivation was to make it easier for reviewers, not about what gets merged. I'll try to not use force pushes here in future.

@mjpieters
Copy link
Contributor Author

The failing stubtest is already reported as #9127, and I think now depends on a new release of Redis with redis/redis-py#2729 included?

@AlexWaygood
Copy link
Member

The failing stubtest is already reported as #9127, and I think now depends on a new release of Redis with redis/redis-py#2729 included?

Yes, I think it would be fine just to add an allowlist entry for now!

The original location,`redis.asyncio.cluster.RedisCluster
`,  is already listed here.
@mjpieters
Copy link
Contributor Author

Yes, I think it would be fine just to add an allowlist entry for now!

I saw the original redis.asyncio.cluster.RedisCluster location was already in that list, only the new export needed adding.

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks!

@AlexWaygood AlexWaygood merged commit 03e3955 into python:main May 6, 2023
50 checks passed
@mjpieters mjpieters deleted the redis_updates branch May 7, 2023 13:34
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.

None yet

2 participants