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.

Sign up for GitHub

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

Jump to bottom

Fix exchange type enum type #1410

Merged
merged 2 commits into from
Dec 27, 2022
Merged

Fix exchange type enum type #1410

merged 2 commits into from
Dec 27, 2022

Conversation

liortct
Copy link
Contributor

@liortct liortct commented Dec 27, 2022

Addressing #1409 .
Fix ExchangeType enum type to str.

@lukebakken lukebakken self-assigned this Dec 27, 2022
@lukebakken lukebakken added this to the 2.0.0 milestone Dec 27, 2022
lukebakken
lukebakken requested changes Dec 27, 2022
View reviewed changes
Copy link
Member

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

Please see this comment, thanks - #1409 (comment)

@liortct
Copy link
Contributor Author

liortct commented Dec 27, 2022

Please see this comment, thanks - #1409 (comment)

You are right but StrEnum is new in Python 3.11.
I want this will work not only for Python 3.11 so I don't use StrEnum and simply did like here and inheritance from str.

@lukebakken
Copy link
Member

Argh that's annoying.

Could you try to do an import, catch the ImportError, and fix things up? Otherwise at some point I'm going to forget that StrEnum is available and never update the code.

I'd rather do that than check the python version.

Thanks!

@liortct
Copy link
Contributor Author

liortct commented Dec 27, 2022

Argh that's annoying.

Could you try to do an import, catch the ImportError, and fix things up? Otherwise at some point I'm going to forget that StrEnum is available and never update the code.

I'd rather do that than check the python version.

Thanks!

Fixed :)

lukebakken
lukebakken approved these changes Dec 27, 2022
View reviewed changes
Copy link
Member

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

Thanks!

@liortct
Copy link
Contributor Author

liortct commented Dec 27, 2022

Thanks!

Can you merge this PR?

@lukebakken
Copy link
Member

Can you merge this PR?

Yes, I was waiting for the checks to finish.

@lukebakken lukebakken merged commit 49fea4c into pika:main Dec 27, 2022
@lukebakken lukebakken modified the milestones: 2.0.0, 1.3.2 Dec 27, 2022
@lukebakken lukebakken mentioned this pull request Dec 27, 2022
Closed
lukebakken added a commit that referenced this pull request Dec 27, 2022
@lukebakken
Merge pull request #1410 from liortct/exchange-type-string
5bd1db8 
Fix exchange type enum type

(cherry picked from commit 49fea4c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukebakken lukebakken lukebakken approved these changes

Assignees

@lukebakken lukebakken

Labels
None yet
Projects
None yet
Milestone
1.3.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@liortct @lukebakken