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.

Sign up for GitHub

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

Jump to bottom

bluezdbus: Allow hardcoding a UID with BLEAK_DBUS_AUTH_UID #1182

Merged
merged 2 commits into from
Dec 27, 2022
Merged

bluezdbus: Allow hardcoding a UID with BLEAK_DBUS_AUTH_UID #1182

merged 2 commits into from
Dec 27, 2022

Conversation

kiwiz
Copy link
Contributor

@kiwiz kiwiz commented Dec 23, 2022

Closes #1181

@kiwiz kiwiz marked this pull request as ready for review December 23, 2022 21:55
@bdraco
Copy link
Contributor

bdraco commented Dec 24, 2022

Don't forget the bump the minimum dbus-fast version

@bdraco
Copy link
Contributor

bdraco commented Dec 24, 2022

Also if you want to use it in Home Assistant its dbus-fast is hard pinned so you'll need to do a PR to bump that as well https://github.com/home-assistant/core/blob/dev/homeassistant/components/bluetooth/manifest.json#L14

dlech
dlech reviewed Dec 24, 2022
View reviewed changes
bleak/backends/bluezdbus/utils.py Outdated Show resolved Hide resolved
docs/backends/linux.rst Outdated
@@ -41,6 +41,12 @@ individual Bleak objects should not be shared between event loops. Otherwise,
RuntimeErrors similar to ``[...] got Future <Future pending> attached to a
different loop`` will be thrown.

User Namespaces
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have another similar request for anonymous d-bus authentication at #796 that was never resolved. So perhaps it makes sense to make this section about "D-Bus authentication" in general instead of just user namespaces. Then we can add to it in the other PR.

Also, to start a pattern, perhaps we could make this environment variable BLEAK_DBUS_AUTH_UUID and use BLEAK_DBUS_AUTH_ANON in the other PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, will do!

To clarify, do you mean BLEAK_DBUS_AUTH_UID (vs UUID)?

Copy link
Collaborator

@dlech dlech Dec 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that we should replace BLEAK_DBUS_UUID with BLEAK_DBUS_AUTH_UUID everywhere in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I get that - but do you explicitly want it to be U U I D?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, oops, didn't even notice that. Should be UID, of course!

@kiwiz kiwiz changed the title bluezdbus: Add BLEAK_DBUS_UID override bluezdbus: Allow hardcoding a UID with BLEAK_DBUS_AUTH_UID Dec 24, 2022
@kiwiz kiwiz force-pushed the develop branch 3 times, most recently from 4c4d290 to 81283ce Compare December 24, 2022 21:09
@kiwiz
Copy link
Contributor Author

kiwiz commented Dec 24, 2022

Tested with:

sudo docker run --rm -it --entrypoint /bin/bash -v /run/dbus:/run/dbus:ro python:3
# In container
pip install https://github.com/kiwiz/bleak/archive/refs/heads/develop.zip
BLEAK_DBUS_AUTH_UID=800000 python -c 'import bleak; import asyncio; asyncio.run(bleak.BleakScanner.discover())'

@kiwiz
Copy link
Contributor Author

kiwiz commented Dec 24, 2022

My bad - poetry.lock should be up-to-date now.

@dlech
Copy link
Collaborator

dlech commented Dec 24, 2022

Can you run poetry lock --no-update instead so it only updates the one package?

@kiwiz
Copy link
Contributor Author

kiwiz commented Dec 24, 2022

Hmm, that appears to generate the same file. Could this be due to an update to the poetry.lock format? I'm running poetry v1.3.1

@dlech
Copy link
Collaborator

dlech commented Dec 26, 2022

OK, I guess there is a new version of Poetry. Let's do a separate commit to update the lock file first.

@kiwiz
Copy link
Contributor Author

kiwiz commented Dec 27, 2022

Split out poetry.lock format update into a separate commit.

kiwiz
kiwiz commented Dec 27, 2022
View reviewed changes
poetry.lock
Comment on lines -304 to -307
async-timeout = ">=3.0.0"

[package.extras]
docs = ["Sphinx (>=5.1.1,<6.0.0)", "myst-parser (>=0.18.0,<0.19.0)", "sphinx-rtd-theme (>=1.0.0,<2.0.0)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)", "sphinxcontrib-fulltoc (>=1.2.0,<2.0.0)"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was removed by poetry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. These were marked as deps of dbus-fast in 1.22.0.

@dlech dlech merged commit 4a6d1c4 into hbldh:develop Dec 27, 2022
@dlech
Copy link
Collaborator

dlech commented Dec 27, 2022

Nice work, thanks!

@dlech dlech mentioned this pull request Dec 27, 2022
Open
@dlech dlech mentioned this pull request Mar 17, 2023
Merged
@kiwiz kiwiz mentioned this pull request Mar 18, 2023
Closed
20 tasks
@kiwiz kiwiz mentioned this pull request Mar 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlech dlech dlech left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hardcoding DBus user UID
3 participants
@kiwiz @bdraco @dlech