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

[WIP] active mode support #38

Closed
wants to merge 1 commit into from
Closed

[WIP] active mode support #38

wants to merge 1 commit into from

Conversation

rsichnyi
Copy link
Contributor

as per #16

this works, but still is far away from being suitable for merge...

TODO:

  • decide what to do with passive_server in case of active mode (await asyncio.sleep(0) is stupid)
  • add client-side support
  • add tests
  • release data port if needed when closing passive server on mode switch
  • catch and correctly process ConnectionRefusedError for data connection in active mode

host = str.join(".", map(str, nums[:4]))
port = (nums[4] << 8) | nums[5]

if not connection.future.passive_server.done():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't get this check. Passive server already closed and removed from connection above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need connection.futures.passive_server to pass ConnectionCondition(passive_server_started) checks

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see… I think we need to extend ConnectionCondition logic. Cause making fake passive server is not good, I think.

@rsichnyi
Copy link
Contributor Author

I believed that i will have time to work on this but seems like not, closing as it's really outdated.

@rsichnyi rsichnyi closed this Dec 13, 2016
@pohmelie
Copy link
Collaborator

Don't worry, since I definitely will rewrite aioftp as sioftp (sans-io ftp) with slightly different architecture there will be place for active mode 😉

@ramstein74
Copy link

I really need active mode support.
What is the status of this feature ?

@pohmelie
Copy link
Collaborator

pohmelie commented Feb 7, 2018

@ramstein74
This pull is outdated.

Active mode is rarely used. What is the reason you can't use EPSV/PASV instead?

@ramstein74
Copy link

ramstein74 commented Feb 7, 2018 via email

@pohmelie
Copy link
Collaborator

pohmelie commented Feb 7, 2018

Do you know the reason why PASV do not work? How it looks from client side?

@ramstein74
Copy link

ramstein74 commented Feb 7, 2018 via email

@pohmelie
Copy link
Collaborator

pohmelie commented Feb 7, 2018

Ok, so we can see both EPSV and PASV are not implemented. Very uncommon case, honestly. There is nothing to do with aioftp to make it work right now. I suggest you to use ftplib, since implement PORT for aioftp is long road.

@ramstein74
Copy link

ramstein74 commented Feb 7, 2018 via email

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.

3 participants