FTP - Add proxy support to FTP, SFTP, and FTPS connectors #1907
Conversation
* Add optional proxy to the three FTPSettings case classes * Apply the proxy to the underlying client before connection is made * Add squid proxy to docker compose for testing * Add test classes for each of the three protocols
The hbase dependency uses an old version of hadoop-common, which in turns uses an old version of commons-net which is getting picked up in the unidoc task in favour of the later commons-net lib.
probot-autolabeler
bot
added
the
dependency-change
|
The build is now failing on the MiMa check. Could someone from the core team confirm I'm ok to add an exception for this change? |
|
Also, the unidoc task was failing because is was picking up an old version of commons-net from the hbase dependency. I've fixed it by upgrading that dependency, but perhaps there's a better way to do this? |
* Added commons-net filter to the unidoc task. * Revert Dependencies to origin/master
|
Hi @ennru, thanks for the pointers. I've made the changes you suggested. My FTP tests are passing, but there are a few failing modules. I can't reproduce the failures locally, and they seem unrelated to my changes. Do you have any tips to get the tests green? |
|
Also, I think the dependency-change flag can be removed now. |
| ) | ||
|
|
||
| override def toString = | ||
| s"""FtpSettings(host=$host,port=$port,credentials=$credentials,binary=$binary,passiveMode=$passiveMode,configureConnection=$configureConnection)""" | ||
| s"""FtpSettings(host=$host,port=$port,credentials=$credentials,binary=$binary,passiveMode=$passiveMode,configureConnection=$configureConnection),proxy=$proxy""" |
There was a problem hiding this comment.
The ) should move to the end. You may make this a single-" string and split it with one field per line, so this doesn't happen again.
| ) | ||
|
|
||
| override def toString = | ||
| s"""FtpsSettings(host=$host,port=$port,credentials=$credentials,binary=$binary,passiveMode=$passiveMode,configureConnection=$configureConnection)""" | ||
| s"""FtpsSettings(host=$host,port=$port,credentials=$credentials,binary=$binary,passiveMode=$passiveMode,configureConnection=$configureConnection),proxy=$proxy""" |
ennru
removed
the
dependency-change
docs/src/main/paradox/ftp.md
Outdated
| @@ -37,6 +37,8 @@ The example demonstrates optional use of `configureConnection` option available | |||
|
|
|||
| For non-anonymous connection, please provide an instance of @scaladoc[NonAnonFtpCredentials](akka.stream.alpakka.ftp.FtpCredentials$$NonAnonFtpCredentials) instead. | |||
|
|
|||
| For connection via a proxy, please provide an instance of `java.net.Proxy`. | |||
There was a problem hiding this comment.
Ah, there is this. Maybe refer to the withProxy method.
Co-Authored-By: Enno <458526+ennru@users.noreply.github.com>
Co-Authored-By: Enno <458526+ennru@users.noreply.github.com>
Co-Authored-By: Enno <458526+ennru@users.noreply.github.com>
|
Good catch on the copy method. I've committed your suggestions. |
|
Thank you! Great to see this long-standing feature request fixed. |
Purpose
Adds proxy support to FTP, SFTP, and FTPS connectors
References
References #600
Changes
withProxymethod on all three FTPSettings builders that takes ajava.net.ProxyobjectBackground Context
The approach is relatively straight forward. The extra option fits neatly into to existing builder pattern and passing the proxy to the client at the start of the connect operation is the natural place for it to go.
I've chosen to use a squid proxy in the testing as this is a widely used proxy.