Add support for multiple addresses when publishing a service.#27
Closed
basilfx wants to merge 4 commits intopython-zeroconf:masterfrom
Closed
Add support for multiple addresses when publishing a service.#27basilfx wants to merge 4 commits intopython-zeroconf:masterfrom
basilfx wants to merge 4 commits intopython-zeroconf:masterfrom
Conversation
Collaborator
|
@basilfx Thank you for the contribution. One thing I'm not sure about is the fact that |
Author
|
Maybe something like this could keep it compatible with the current API (if added to ServiceInfo)? This would ensure that you can still get and set to the address property. @property
def address(self):
return self.addresses[0] if self.addresses else None
@address.setter
def address(self, value):
if value is None:
self.addresses = []
else:
if value in self.addresses:
self.addresses.remove(value)
# Ensure address is at index 0 so `self.address == value`.
self.addresses.insert(0, value)Haven't tested it :-) |
Collaborator
|
Yes, I'm also thinking about something along these lines :) |
Collaborator
dtantsur
added a commit
to dtantsur/python-zeroconf
that referenced
this pull request
May 20, 2019
* Raise deprecation warnings when address is used * Add a compatibility property to avoid breaking existing code (based on suggestion by Bas Stottelaar in PR python-zeroconf#27) * Make addresses keyword-only, so that address can be eventually removed and replaced with it without breaking consumers * Raise TypeError instead of an assertion on conflicting address and addresses
dtantsur
added a commit
to dtantsur/python-zeroconf
that referenced
this pull request
May 20, 2019
* Raise deprecation warnings when address is used * Add a compatibility property to avoid breaking existing code (based on suggestion by Bas Stottelaar in PR python-zeroconf#27) * Make addresses keyword-only, so that address can be eventually removed and replaced with it without breaking consumers * Raise TypeError instead of an assertion on conflicting address and addresses
Collaborator
|
Update: I've posted #170 with an updated version of this PR. |
dtantsur
added a commit
to dtantsur/python-zeroconf
that referenced
this pull request
May 20, 2019
* Raise deprecation warnings when address is used * Add a compatibility property to avoid breaking existing code (based on suggestion by Bas Stottelaar in PR python-zeroconf#27) * Make addresses keyword-only, so that address can be eventually removed and replaced with it without breaking consumers * Raise TypeError instead of an assertion on conflicting address and addresses
dtantsur
added a commit
to dtantsur/python-zeroconf
that referenced
this pull request
May 20, 2019
* Raise deprecation warnings when address is used * Add a compatibility property to avoid breaking existing code (based on suggestion by Bas Stottelaar in PR python-zeroconf#27) * Make addresses keyword-only, so that address can be eventually removed and replaced with it without breaking consumers * Raise TypeError instead of an assertion on conflicting address and addresses
jstasiak
pushed a commit
that referenced
this pull request
May 22, 2019
This is a rebased and fixed version of PR #27, which also adds compatibility shim for ServiceInfo.address and does a proper deprecation for it. * Present all addresses that are available. * Add support for publishing multiple addresses. * Add test for backwards compatibility. * Provide proper deprecation of the "address" argument and field * Raise deprecation warnings when address is used * Add a compatibility property to avoid breaking existing code (based on suggestion by Bas Stottelaar in PR #27) * Make addresses keyword-only, so that address can be eventually removed and replaced with it without breaking consumers * Raise TypeError instead of an assertion on conflicting address and addresses * Disable black on ServiceInfo.__init__ until black is fixed Due to psf/black#759 black produces code that is invalid Python 3.5 syntax even with --target-version py35. This patch disables reformatting for this call (it doesn't seem to be possible per line) until it's fixed.
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
I use this module to advertise my own service, and noticed that it does not allow multiple addresses (TYPE_A records) when advertising. Services such as iTunes do this.
This PR adds support for it, without breaking existing implementations.
ServiceInfonow has a list namedaddresses, instead ofaddress, but still allowsaddressparameter. It supports publishing, but also browsing will show multiple addresses.To illustrate:
(This app lists IP addresses twice. It's not a cause of this PR.)
Kind regards,
Bas