Skip to content

Remove string support in api_version #1814

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

Closed
wants to merge 1 commit into from

Conversation

jeffwidman
Copy link
Contributor

@jeffwidman jeffwidman commented May 24, 2019

This breaks backwards compatibility, probably best not to merge 'til 2.0 release

A long time ago, api_version supported strings. That has been
deprecated for years in favor of tuples. Time to remove support for the
strings.


This change is Reviewable

@jeffwidman jeffwidman added this to the 2.0 milestone May 24, 2019
@jeffwidman jeffwidman requested a review from dpkp May 24, 2019 06:30
@jeffwidman jeffwidman force-pushed the Remove-support-for-api_version-strings branch from eb33f61 to c463b1c Compare May 24, 2019 07:17
A long time ago, `api_version` supported strings. That has been
deprecated for years in favor of tuples. Time to remove support for the
strings.
@jeffwidman jeffwidman force-pushed the Remove-support-for-api_version-strings branch from c463b1c to b6c6a27 Compare May 24, 2019 07:17
@dpkp
Copy link
Owner

dpkp commented May 29, 2019

What do we gain by removing this? I understand the desire to keep code clean, but is there a specific feature that is blocked by keeping this in ?

@jeffwidman
Copy link
Contributor Author

Agreed. And personally I think the string value is a sensible enough choice, even though it is error prone.

I also noticed that confluent-kafka-python uses this config value as a string, so I'd rather keep us supporting a similar interface for folks switching back and forth.

@jeffwidman jeffwidman closed this Aug 15, 2019
@jeffwidman jeffwidman deleted the Remove-support-for-api_version-strings branch August 15, 2019 19:09
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.

2 participants