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

Add feature flag to make the remote state verification backwards compatible again #491

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

purbon
Copy link
Collaborator

@purbon purbon commented Apr 26, 2022

to be removed in future versions and make true by default.

@purbon
Copy link
Collaborator Author

purbon commented Apr 26, 2022

@akselh @sverrehu This should bring this feature backwards compatible again, as it should have always been ;-) sorry for the hassle. I will make a new version rather soon, sorry been lately slammed by work.

@sverrehu
Copy link
Contributor

Would it be better to print the warning in one, central place? Then you would avoid both duplicated code and "spammed" logs.

@purbon
Copy link
Collaborator Author

purbon commented Apr 26, 2022

@sverrehu probably true, but I kinda need this change soon for a project ;-) ... I will improve the code in next iterations as this central place would require some reworking of the class structure (totally need anyway).

@purbon purbon merged commit ea7eefd into master Apr 26, 2022
purbon added a commit that referenced this pull request Apr 26, 2022
purbon added a commit that referenced this pull request Apr 27, 2022
@akselh
Copy link
Contributor

akselh commented Apr 27, 2022

Thanks for fixing this issue @purbon!

When this is later removed it is important to handle the existing features of config ALLOW_XXX being false. For example in the case ALLOW_DELETE_TOPICS=false the normal flow of work will be to remove the topic from the topology first and then remove it from the cluster.

@purbon
Copy link
Collaborator Author

purbon commented Apr 27, 2022 via email

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.

3 participants