[go: nahoru, domu]
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

Incoming support Transport::remote_addr #713

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Incoming support Transport::remote_addr #713

wants to merge 30 commits into from

Conversation

valbendan
Copy link
Contributor

The current run_incoming does not support obtaining remote_addr dute to impl (PS: always return None).

impl<T: AsyncRead + AsyncWrite + Unpin> Transport for LiftIo<T> {
    fn remote_addr(&self) -> Option<SocketAddr> {
        None
    }
}

After some investigated, I think the most elegant solution should be to use specialization, so that user can specialize his own type override the default impl.

/// re-impl if specialization is available
impl<T: AsyncRead + AsyncWrite + Unpin> Transport for LiftIo<T> {
    default fn remote_addr(&self) -> Option<SocketAddr> {
        None
    }
}

Unfortunatly, specialization is not available in stable channel and will not be available in foreseeable future.

So I write a dirty copy-and-paste impl.

@seanmonstar
Copy link
Owner

I agree, specialization would be the best solution. It might be available in 2034...

I'd be fine with exposing the Transport trait, and then changing run_incoming to requiring it. To aid with plugging in types for uses who just don't care, we could also expose LiftIo. I think the only thing is requiring it on run_incoming would be a breaking change, so should be part of 0.3?

@valbendan
Copy link
Contributor Author

If you do not mind break compatibility, I'd like to make a new PR based on your proposation.

The reason why I implement this by adding v2 is because I am worried about compatibility.

Do you have any roadmap/idea for when we will see the version 0.3 ?

@seanmonstar
Copy link
Owner

We could release a v0.3 soon. There's a milestone/tracking issue for breaking changes. I just don't have the time to do them all.

@valbendan
Copy link
Contributor Author

New PR is comming.

Can we merge this into version 0.3 ?

Since you don't have time to deal with everything at once, do you mind releasing the preview version first (eg: 0.3-alpha1)?

@mcginty mcginty mentioned this pull request Apr 5, 2021
Open
@ktff ktff mentioned this pull request May 1, 2021
Open
@jxs jxs added the waiting-on-reviewer Awaiting review from the assignee but also interested parties. label Jun 10, 2021
@dholroyd dholroyd mentioned this pull request Nov 21, 2021
Open
@wfraser wfraser mentioned this pull request Apr 29, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
waiting-on-reviewer Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@valbendan @seanmonstar @jxs