Uses a sane default for pysftp_conn_kwargs

[daveFNbuck]

Description

pysftp_conn_kwargs defaults to None, which causes a TypeError when passed
with ** for the kwargs of pysftp.connect. This fixes replaces None with {} when storing pysftp_conn_kwargs in the RemoteFileSystem.

Motivation and Context

I found that since updating to the latest luigi yesterday, my sftp tasks have stopped working. There would be an error like TypeError: type object argument after ** must be a mapping, not NoneType in the luigi ftp library. I traced it down to the default value for the new optional argument pysftp_conn_kwargs in RemoteFileSystem. By storing {} when None is passed, the error is fixed.

Have you tested this? If so, how?

I ran a previously failing job with this code and it succeeded.

[mention-bot]

@daveFNbuck, thanks for your PR! By analyzing the annotation information on this pull request, we identified @petergaultney, @dlstadther and @ajornetic3 to be potential reviewers

[petergaultney]

eesh, that's embarrassing. good catch. +1.

I guess this is what you get for contributing code and never using the default.

[daveFNbuck]

No worries, it was a pretty easy fix. Unfortunately, I think this made it into the 2.3.0 release, so we'll probably want to bump to 2.3.1. This can be a fairly costly bug, as google recently started requiring sftp for some ad-related uploads.

[petergaultney]

correct me if I'm wrong, but I was looking at the tests today and it seemed like luigi doesn't have any tests in the suite for the SFTP component of RemoteTarget? I think I was looking at ftp_test and there were only FTP tests in there. That might be exacerbating the issue.

[Tarrasch]

@daveFNbuck. Do you think I should merge this immediately and release 2.3.1?

[daveFNbuck]

@Tarrasch I think so. Loss of sftp can be pretty critical.
@petergaultney agreed, we should add some sftp tests. I was just blindly using it until today :)