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 back api._twisted_publish with deprecation warning #375

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mattiaverga
Copy link

@mattiaverga mattiaverga commented Jun 21, 2024

api._twisted_publish is (was) used for mocking tests.
The removal of that method in 3.6.0 without previous warnings broke Bodhi tests (and I think other dependent packages too). The removal should be handled by provide a proper warning when the method is used and wait a decent amount of time before complete removal.

This PR adds back the deprecated method, which just emits a deprecation notice and routes to the new method, which seems 1:1 compatible.
I just don't know if the decorators should be applied to the deprecated method as well.

@mattiaverga mattiaverga force-pushed the 3.6-deprecated branch 2 times, most recently from aa24495 to 51bcaf4 Compare June 22, 2024 13:04
@abompard
Copy link
Member

Ah, sorry about the breakage, I assumed that functions starting with an underscore would be private and people woudn't use them directly, but it's true that it can easily happen when mocking.
The fedora_messaging.testing module comes with functions and fixtures to help with testing, is there something that you'd need me to add there? I'd rather provide the testing functionality as part of fedora_messaging rather that force people to mock private functions.

@mattiaverga
Copy link
Author

The reason behind mocking api._twisted_publish is to ensure a proper failure message if, for some reason, a message publish was not properly handled during tests.
See fedora-infra/bodhi@3eb3204

But I originally created this PR because I was quite sure to have seen an example about mocking that method in fedora-messaging docs (which, however, I cannot find anymore...).

So, I'm unsure if this can be useful to some other places other than bodhi or not. BTW about bodhi I solved the transition with a try / except AttributeError to handle both the new and old fedora-messaging versions.

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