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

dbus: stopping now requires an optional argument profile_switch #551

Closed
jelly opened this issue Aug 21, 2023 · 3 comments · Fixed by #552
Closed

dbus: stopping now requires an optional argument profile_switch #551

jelly opened this issue Aug 21, 2023 · 3 comments · Fixed by #552

Comments

@jelly
Copy link

jelly commented Aug 21, 2023

Since 7f3a79b#diff-f86334dc9215b06e3455f0dd706ce4979971f12d39c11587078c6010dba4ff7fR120 it is no longer possible to call stop without a profile_switch argument.

Previously:

[root@fedora-38-127-0-0-2-2201 ~]# busctl call com.redhat.tuned /Tuned com.redhat.tuned.control stop
b true

Since the last RC

[root@fedora-rawhide-127-0-0-2-2201 ~]# busctl call com.redhat.tuned /Tuned com.redhat.tuned.control stop
Call failed: TypeError: stop() missing 1 required positional argument: 'profile_switch'

I'm wondering if this API change is intentional, or if it should be an optional argument.

BTW, it seems to me that the introspection is wrong now?

.start                         method    -         b            -
.stop                          method    v         b            -
[root@fedora-rawhide-127-0-0-2-2201 ~]# busctl call com.redhat.tuned /Tuned com.redhat.tuned.control stop b false
Call failed: TypeError: stop() got multiple values for argument 'caller'
@yarda
Copy link
Contributor

yarda commented Aug 22, 2023

Thanks, good catch, it was introduced by 7f3a79b.

yarda added a commit to yarda/tuned that referenced this issue Aug 22, 2023
Fixes redhat-performance#551

Signed-off-by: Jaroslav Škarvada <jskarvad@redhat.com>
@yarda
Copy link
Contributor

yarda commented Aug 22, 2023

@RHEmployee it would be great to have gating test for the API to prevent such problems in the future.

@RHEmployee
Copy link
Contributor

@yarda Hi. Would you please describe what exactly do you want to to cover in test?
Should be test aimed only to this issue or should cover wide use?
I'm happy to create downstream test and upstream it.

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 a pull request may close this issue.

3 participants