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

sd-bus: Async properties callbacks #18206

Closed
wants to merge 2 commits into from

Conversation

igo95862
Copy link
Contributor

5320566 this commit makes properties callbacks being able to return positive integer to indicate that the response will be handled async. (just like methods callbacks can)

b58bf3f This commit fixes test cases returning 1 in some cases even though they did not want async handle.

I ran the test cases and they work without issues.

I think there should be a documentation that if you make an async property callback its your responsibility to close the message. I can add that to man page.

Fixes: #18193

@igo95862 igo95862 changed the title Async properties callbacks sd-bus: Async properties callbacks Jan 11, 2021
@poettering
Copy link
Member

Hmm? I don't get this? As reaction to GetAll() we call a bunch of property handlers, how is this patch handling that?

In general, client sides generally assume (in particular gdbus) that properties are "cheap" to acquire, can be cached and cannot fail. Asynchronous behaviour for handling them hence is against these assumption, since that suggests they are slow, require work to acquire and thus are more likely to fail.

hence, the PR doesn't make too much sense to me?

@poettering poettering added needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer sd-bus labels Jan 11, 2021
@igo95862
Copy link
Contributor Author

igo95862 commented Jan 11, 2021

As reaction to GetAll() we call a bunch of property handlers, how is this patch handling that?

Well it does not... (because I forgot about it)

However, there is SD_BUS_VTABLE_PROPERTY_EXPLICIT flag is used ' because the value is large or slow to calculate'. I looked at source code and it seems like 'GetAll' will skip over those methods. Maybe allow async properties only with that flag set?

@igo95862
Copy link
Contributor Author

igo95862 commented Jan 12, 2021

After thinking about it I probably won't need async properties. However, I think it should be documented that properties are always sync. Currently there is no mention of this in man pages. I can submit a patch for man page either in this pull request or a new one.

igo95862 added a commit to python-sdbus/python-sdbus that referenced this pull request Jan 12, 2021
Base automatically changed from master to main January 21, 2021 11:55
@poettering
Copy link
Member

yeah, a patch for the man page would be lovely!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-reporter-feedback ❓ There's an unanswered question, the reporter needs to answer sd-bus
Development

Successfully merging this pull request may close these issues.

sd-bus: Can't asynchronously answer properties Get call?
2 participants