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

client: allow announce custom port #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mayli
Copy link

@mayli mayli commented Oct 8, 2021

No description provided.

@elektito
Copy link
Owner

elektito commented Oct 8, 2021

Thank you for the PR. But I don't quite understand the purpose. We can set the tracker port through the announce_uri parameter of TrackerClient. For example:

client = TrackerClient(local_addr='udp://tracker.example.org:8000')

So there's no reason to pass port to the announce method. If we want to talk to multiple trackers, we should create multiple clients.

@mayli
Copy link
Author

mayli commented Oct 8, 2021

Thank you for the PR. But I don't quite understand the purpose. We can set the tracker port through the announce_uri parameter of TrackerClient. For example:

client = TrackerClient(local_addr='udp://tracker.example.org:8000')

So there's no reason to pass port to the announce method. If we want to talk to multiple trackers, we should create multiple clients.

This is not for the tracker port, it's to tell the client's local port, instead of using a random one allocated from UDP.

@elektito
Copy link
Owner

elektito commented Oct 8, 2021

Just realized that line of code I had there was incorrect. It should be: client = TrackerClient(local_addr='udp://tracker.example.org:8000'). That was also incorrect in the README, which I've fixed.

But if you want is to use a different local port, I'm still not sure how that is supposed to work. We already have a socket with a port assigned to it. If we change the value in the message, the tracker would contact another port. Is your intention to have another socket to receive tracker response on?

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 this pull request may close these issues.

None yet

2 participants