Fix SMBFilesystem - use standard port when no port specified

fsspec/implementations/smb.py

@@ -81,7 +81,7 @@ def __init__(
         ----------
         host: str
             The remote server name/ip to connect to
-        port: int
+        port: int or None
             Port to connect with. Usually 445, sometimes 139.
         username: str or None
             Username to connect with. Required if Kerberos auth is not being used.
@@ -114,12 +114,16 @@ def __init__(
         self.share_access = share_access
         self._connect()
 
+    @property
+    def _port(self):
+        return 445 if self.port is None else self.port
+
     def _connect(self):
         smbclient.register_session(
             self.host,
             username=self.username,
             password=self.password,
-            port=445 if self.port is None else self.port,
+            port=self._port,
             encrypt=self.encrypt,
             connection_timeout=self.timeout,
         )
@@ -139,23 +143,23 @@ def _get_kwargs_from_urls(path):
     def mkdir(self, path, create_parents=True, **kwargs):
         wpath = _as_unc_path(self.host, path)
         if create_parents:
-            smbclient.makedirs(wpath, exist_ok=False, port=self.port, **kwargs)
+            smbclient.makedirs(wpath, exist_ok=False, port=self._port, **kwargs)
         else:
-            smbclient.mkdir(wpath, port=self.port, **kwargs)
+            smbclient.mkdir(wpath, port=self._port, **kwargs)
 
     def makedirs(self, path, exist_ok=False):
         if _share_has_path(path):
             wpath = _as_unc_path(self.host, path)
-            smbclient.makedirs(wpath, exist_ok=exist_ok, port=self.port)
+            smbclient.makedirs(wpath, exist_ok=exist_ok, port=self._port)
 
     def rmdir(self, path):
         if _share_has_path(path):
             wpath = _as_unc_path(self.host, path)
-            smbclient.rmdir(wpath, port=self.port)
+            smbclient.rmdir(wpath, port=self._port)
 
     def info(self, path, **kwargs):
         wpath = _as_unc_path(self.host, path)
-        stats = smbclient.stat(wpath, port=self.port, **kwargs)
+        stats = smbclient.stat(wpath, port=self._port, **kwargs)
         if S_ISDIR(stats.st_mode):
             stype = "directory"
         elif S_ISLNK(stats.st_mode):
@@ -176,18 +180,18 @@ def info(self, path, **kwargs):
     def created(self, path):
         """Return the created timestamp of a file as a datetime.datetime"""
         wpath = _as_unc_path(self.host, path)
-        stats = smbclient.stat(wpath, port=self.port)
+        stats = smbclient.stat(wpath, port=self._port)
         return datetime.datetime.fromtimestamp(stats.st_ctime, tz=datetime.timezone.utc)
 
     def modified(self, path):
         """Return the modified timestamp of a file as a datetime.datetime"""
         wpath = _as_unc_path(self.host, path)
-        stats = smbclient.stat(wpath, port=self.port)
+        stats = smbclient.stat(wpath, port=self._port)
         return datetime.datetime.fromtimestamp(stats.st_mtime, tz=datetime.timezone.utc)
 
     def ls(self, path, detail=True, **kwargs):
         unc = _as_unc_path(self.host, path)
-        listed = smbclient.listdir(unc, port=self.port, **kwargs)
+        listed = smbclient.listdir(unc, port=self._port, **kwargs)
         dirs = ["/".join([path.rstrip("/"), p]) for p in listed]
         if detail:
             dirs = [self.info(d) for d in dirs]
@@ -218,36 +222,36 @@ def _open(
         if "w" in mode and autocommit is False:
             temp = _as_temp_path(self.host, path, self.temppath)
             return SMBFileOpener(
-                wpath, temp, mode, port=self.port, block_size=bls, **kwargs
+                wpath, temp, mode, port=self._port, block_size=bls, **kwargs
             )
         return smbclient.open_file(
             wpath,
             mode,
             buffering=bls,
             share_access=share_access,
-            port=self.port,
+            port=self._port,
             **kwargs,
         )
 
     def copy(self, path1, path2, **kwargs):
         """Copy within two locations in the same filesystem"""
         wpath1 = _as_unc_path(self.host, path1)
         wpath2 = _as_unc_path(self.host, path2)
-        smbclient.copyfile(wpath1, wpath2, port=self.port, **kwargs)
+        smbclient.copyfile(wpath1, wpath2, port=self._port, **kwargs)
 
     def _rm(self, path):
         if _share_has_path(path):
             wpath = _as_unc_path(self.host, path)
-            stats = smbclient.stat(wpath, port=self.port)
+            stats = smbclient.stat(wpath, port=self._port)
             if S_ISDIR(stats.st_mode):
-                smbclient.rmdir(wpath, port=self.port)
+                smbclient.rmdir(wpath, port=self._port)
             else:
-                smbclient.remove(wpath, port=self.port)
+                smbclient.remove(wpath, port=self._port)
 
     def mv(self, path1, path2, **kwargs):
         wpath1 = _as_unc_path(self.host, path1)
         wpath2 = _as_unc_path(self.host, path2)
-        smbclient.rename(wpath1, wpath2, port=self.port, **kwargs)
+        smbclient.rename(wpath1, wpath2, port=self._port, **kwargs)
 
 
 def _as_unc_path(host, path):

fsspec/implementations/tests/test_smb.py

@@ -17,7 +17,8 @@
 # ! pylint: disable=redefined-outer-name,missing-function-docstring
 
 # Test standard and non-standard ports
-port_test = [445, 9999]
+default_port = 445
+port_test = [None, default_port, 9999]
 
 
 def stop_docker(container):
@@ -40,9 +41,9 @@ def smb_params(request):
     # requires docker
     container = "fsspec_smb"
     stop_docker(container)
-    img = "docker run --name {} --detach -p 139:139 -p {}:445 dperson/samba"
-    cfg = " -p -u 'testuser;testpass' -s 'home;/share;no;no;no;testuser'"
-    cmd = img.format(container, request.param) + cfg
+    cfg = "-p -u 'testuser;testpass' -s 'home;/share;no;no;no;testuser'"
+    port = request.param if request.param is not None else default_port
+    cmd = f"docker run --name {container} --detach -p 139:139 -p {port}:445 dperson/samba {cfg}"
     cid = subprocess.check_output(shlex.split(cmd)).strip().decode()
     logger = logging.getLogger("fsspec")
     logger.debug("Container: %s", cid)
@@ -78,7 +79,10 @@ def test_simple(smb_params):
 
 
 def test_with_url(smb_params):
-    smb_url = "smb://{username}:{password}@{host}:{port}/home/someuser.txt"
+    if smb_params["port"] is None:
+        smb_url = "smb://{username}:{password}@{host}/home/someuser.txt"
+    else:
+        smb_url = "smb://{username}:{password}@{host}:{port}/home/someuser.txt"
     fwo = fsspec.open(smb_url.format(**smb_params), "wb")
     with fwo as fwr:
         fwr.write(b"hello")