-
Notifications
You must be signed in to change notification settings - Fork 221
DNS-SD Client Support #13
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
Conversation
MDNS responders will often send extra records before they are requested (eg SRV and TXT records) and not respond to explicit requests for the same record until the 1 second cooldown period. To handle this, cache all records and replay the cache when to new queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the pure mDNS example and add a DNS-SD example, too.
|
Sorry about the delay, hopefully I can review this soon and get it in! |
|
hi @armon, No worries. Let me know if you have any questions/comments about the design or operation. -- Logan |
|
So I think we need to do a few things to merge this in. This breaks a lot of the existing APIs which is problematic since we use it down stream. Ideally, we can add new interfaces without breaking the existing ones. Additionally, we need to add some tests for this to verify the new behavior since a lot of new code is being added. |
|
@armon definitely understandable. I'm actually not working on the project anymore that needed this library, so I don't know when I'll have a chance to make the suggested changes. I did start trying to add tests, but IIRC I there is some limitation where you can't receive multicast from a local multicast server, so that was causing issues. |
|
@lsowen Understandable. I also haven't had time to commit to this project. Let's just leave this open for now. |
|
The fork works as it should be. Would be nice to get this merged. |
|
hi @tcurdt, I think @armon was (rightfully) concerned with my lack of tests and breaking changes. I tried to expose the same methods, but some of the behavior was different. However, I'm glad the code worked for you. If you could help out by improving the test coverage, I'm sure it would be more likely to get merged back in. Thanks! |
|
I think this merge request should not bleed to death, maybe this can be reworked and merged. It very nice to see a RFC implement in golang 👍 |
|
FYI I just created a pull request for DNS-SD support on the server side: #36 |
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
Re-worked mdns client to cache all over the wire results, replay the cache for a new query, and provide additional RFC6763 data to lookups (eg parsing TXT records). Additionally, mdns lookups can be made directly for a record (eg a SRV record) instead of only being able to resolve a service.