Skip to content

Commit 296b2c5

Browse files
committed
Fold Path/prefix setup into walk_and_upload and resolve pylint findings
* Move the ``Path`` / ``is_dir`` check / prefix rstrip into ``walk_and_upload``; it now returns an ``UploadDirResult`` with the resolved source and normalised prefix so each backend can still log its own line. Eliminates the last duplicate-code finding across the four cloud/SFTP ``*_upload_dir`` functions. * Split ``build_default_registry`` into ``_local_commands`` / ``_http_commands`` / ``_drive_commands`` / ``_register_cloud_backends`` helpers so the top-level function drops back under the too-many-locals threshold. * ``SFTPClient.later_init`` now takes an ``SFTPConnectOptions`` dataclass (with a kwargs-compat fallback) so it no longer trips too-many-args. * Rename the tqdm bar variable in ``http_download`` off the disallowed ``bar`` name; add a ``.pylintrc`` that teaches pylint about PySide6 as an extension package and about googleapiclient's dynamic ``Resource`` members; silence ``arguments-differ`` / ``useless-import-alias`` at their intentional use sites.
1 parent 5da2ddb commit 296b2c5

11 files changed

Lines changed: 222 additions & 138 deletions

File tree

.pylintrc

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
[MAIN]
2+
# PySide6 is a C-extension binding — pylint's static inference cannot see the
3+
# generated classes, so treat it as a trusted extension package.
4+
extension-pkg-allow-list=PySide6,PySide6.QtCore,PySide6.QtGui,PySide6.QtWidgets
5+
6+
[MESSAGES CONTROL]
7+
# Disabled rules, with rationale:
8+
# C0114/C0115/C0116 — docstring requirements are enforced by review, not lint.
9+
# C0415 — ``import-outside-toplevel`` is used deliberately for
10+
# lazy imports of heavy / optional modules (see CLAUDE.md).
11+
# R0903 — too-few-public-methods; dataclasses and frozen option
12+
# objects are allowed to have no methods.
13+
# W0511 — TODO/FIXME markers; tracked via issues, not lint.
14+
disable=
15+
C0114,
16+
C0115,
17+
C0116,
18+
C0415,
19+
R0903,
20+
W0511
21+
22+
[TYPECHECK]
23+
# googleapiclient's ``Resource`` object exposes ``files()``, ``permissions()``,
24+
# etc. dynamically — pylint can't see them, so whitelist the names rather than
25+
# littering the Drive ops modules with ``# pylint: disable=no-member``.
26+
generated-members=files,permissions
27+
28+
[DESIGN]
29+
# Align with CLAUDE.md's code-quality checklist.
30+
max-args=7
31+
max-locals=15
32+
max-returns=8
33+
max-branches=15
34+
max-statements=50
35+
max-attributes=17
36+
max-public-methods=25
37+
38+
[FORMAT]
39+
max-line-length=100

automation_file/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@
9090
from automation_file.utils.file_discovery import get_dir_files_as_list
9191

9292
if TYPE_CHECKING:
93-
from automation_file.ui.launcher import launch_ui as launch_ui
93+
from automation_file.ui.launcher import (
94+
launch_ui as launch_ui, # pylint: disable=useless-import-alias
95+
)
9496

9597
# Shared callback executor + package loader wired to the shared registry.
9698
callback_executor: CallbackExecutor = CallbackExecutor(executor.registry)

automation_file/core/action_registry.py

Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,35 @@ def event_dict(self) -> dict[str, Command]:
6565
return self._commands
6666

6767

68-
def build_default_registry() -> ActionRegistry:
69-
"""Return a registry pre-populated with every built-in ``FA_*`` action."""
68+
def _local_commands() -> dict[str, Command]:
7069
from automation_file.local import dir_ops, file_ops, zip_ops
71-
from automation_file.remote import http_download
72-
from automation_file.remote.azure_blob import register_azure_blob_ops
73-
from automation_file.remote.dropbox_api import register_dropbox_ops
70+
71+
return {
72+
# Files
73+
"FA_create_file": file_ops.create_file,
74+
"FA_copy_file": file_ops.copy_file,
75+
"FA_rename_file": file_ops.rename_file,
76+
"FA_remove_file": file_ops.remove_file,
77+
"FA_copy_all_file_to_dir": file_ops.copy_all_file_to_dir,
78+
"FA_copy_specify_extension_file": file_ops.copy_specify_extension_file,
79+
# Directories
80+
"FA_copy_dir": dir_ops.copy_dir,
81+
"FA_create_dir": dir_ops.create_dir,
82+
"FA_remove_dir_tree": dir_ops.remove_dir_tree,
83+
"FA_rename_dir": dir_ops.rename_dir,
84+
# Zip
85+
"FA_zip_dir": zip_ops.zip_dir,
86+
"FA_zip_file": zip_ops.zip_file,
87+
"FA_zip_info": zip_ops.zip_info,
88+
"FA_zip_file_info": zip_ops.zip_file_info,
89+
"FA_set_zip_password": zip_ops.set_zip_password,
90+
"FA_unzip_file": zip_ops.unzip_file,
91+
"FA_read_zip_file": zip_ops.read_zip_file,
92+
"FA_unzip_all": zip_ops.unzip_all,
93+
}
94+
95+
96+
def _drive_commands() -> dict[str, Command]:
7497
from automation_file.remote.google_drive import (
7598
client,
7699
delete_ops,
@@ -80,58 +103,51 @@ def build_default_registry() -> ActionRegistry:
80103
share_ops,
81104
upload_ops,
82105
)
106+
107+
return {
108+
"FA_drive_later_init": client.driver_instance.later_init,
109+
"FA_drive_search_all_file": search_ops.drive_search_all_file,
110+
"FA_drive_search_field": search_ops.drive_search_field,
111+
"FA_drive_search_file_mimetype": search_ops.drive_search_file_mimetype,
112+
"FA_drive_upload_dir_to_folder": upload_ops.drive_upload_dir_to_folder,
113+
"FA_drive_upload_to_folder": upload_ops.drive_upload_to_folder,
114+
"FA_drive_upload_dir_to_drive": upload_ops.drive_upload_dir_to_drive,
115+
"FA_drive_upload_to_drive": upload_ops.drive_upload_to_drive,
116+
"FA_drive_add_folder": folder_ops.drive_add_folder,
117+
"FA_drive_share_file_to_anyone": share_ops.drive_share_file_to_anyone,
118+
"FA_drive_share_file_to_domain": share_ops.drive_share_file_to_domain,
119+
"FA_drive_share_file_to_user": share_ops.drive_share_file_to_user,
120+
"FA_drive_delete_file": delete_ops.drive_delete_file,
121+
"FA_drive_download_file": download_ops.drive_download_file,
122+
"FA_drive_download_file_from_folder": download_ops.drive_download_file_from_folder,
123+
}
124+
125+
126+
def _http_commands() -> dict[str, Command]:
127+
from automation_file.remote import http_download
128+
129+
return {"FA_download_file": http_download.download_file}
130+
131+
132+
def _register_cloud_backends(registry: ActionRegistry) -> None:
133+
from automation_file.remote.azure_blob import register_azure_blob_ops
134+
from automation_file.remote.dropbox_api import register_dropbox_ops
83135
from automation_file.remote.s3 import register_s3_ops
84136
from automation_file.remote.sftp import register_sftp_ops
85137

86-
registry = ActionRegistry()
87-
registry.register_many(
88-
{
89-
# Files
90-
"FA_create_file": file_ops.create_file,
91-
"FA_copy_file": file_ops.copy_file,
92-
"FA_rename_file": file_ops.rename_file,
93-
"FA_remove_file": file_ops.remove_file,
94-
"FA_copy_all_file_to_dir": file_ops.copy_all_file_to_dir,
95-
"FA_copy_specify_extension_file": file_ops.copy_specify_extension_file,
96-
# Directories
97-
"FA_copy_dir": dir_ops.copy_dir,
98-
"FA_create_dir": dir_ops.create_dir,
99-
"FA_remove_dir_tree": dir_ops.remove_dir_tree,
100-
"FA_rename_dir": dir_ops.rename_dir,
101-
# Zip
102-
"FA_zip_dir": zip_ops.zip_dir,
103-
"FA_zip_file": zip_ops.zip_file,
104-
"FA_zip_info": zip_ops.zip_info,
105-
"FA_zip_file_info": zip_ops.zip_file_info,
106-
"FA_set_zip_password": zip_ops.set_zip_password,
107-
"FA_unzip_file": zip_ops.unzip_file,
108-
"FA_read_zip_file": zip_ops.read_zip_file,
109-
"FA_unzip_all": zip_ops.unzip_all,
110-
# HTTP
111-
"FA_download_file": http_download.download_file,
112-
# Google Drive
113-
"FA_drive_later_init": client.driver_instance.later_init,
114-
"FA_drive_search_all_file": search_ops.drive_search_all_file,
115-
"FA_drive_search_field": search_ops.drive_search_field,
116-
"FA_drive_search_file_mimetype": search_ops.drive_search_file_mimetype,
117-
"FA_drive_upload_dir_to_folder": upload_ops.drive_upload_dir_to_folder,
118-
"FA_drive_upload_to_folder": upload_ops.drive_upload_to_folder,
119-
"FA_drive_upload_dir_to_drive": upload_ops.drive_upload_dir_to_drive,
120-
"FA_drive_upload_to_drive": upload_ops.drive_upload_to_drive,
121-
"FA_drive_add_folder": folder_ops.drive_add_folder,
122-
"FA_drive_share_file_to_anyone": share_ops.drive_share_file_to_anyone,
123-
"FA_drive_share_file_to_domain": share_ops.drive_share_file_to_domain,
124-
"FA_drive_share_file_to_user": share_ops.drive_share_file_to_user,
125-
"FA_drive_delete_file": delete_ops.drive_delete_file,
126-
"FA_drive_download_file": download_ops.drive_download_file,
127-
"FA_drive_download_file_from_folder": download_ops.drive_download_file_from_folder,
128-
}
129-
)
130-
# Cloud / SFTP backends are first-class; register them on every default registry.
131138
register_s3_ops(registry)
132139
register_azure_blob_ops(registry)
133140
register_dropbox_ops(registry)
134141
register_sftp_ops(registry)
142+
143+
144+
def build_default_registry() -> ActionRegistry:
145+
"""Return a registry pre-populated with every built-in ``FA_*`` action."""
146+
registry = ActionRegistry()
147+
registry.register_many(_local_commands())
148+
registry.register_many(_http_commands())
149+
registry.register_many(_drive_commands())
150+
_register_cloud_backends(registry)
135151
file_automation_logger.info(
136152
"action_registry: built default registry with %d commands", len(registry)
137153
)
Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,63 @@
11
"""Shared directory-walker for cloud/SFTP ``*_upload_dir`` operations.
22
3-
Every backend implements the same pattern: iterate ``Path.rglob('*')``,
4-
skip non-files, compute a POSIX-relative remote identifier, call
3+
Every backend implements the same pattern: validate that ``dir_path``
4+
exists, normalise the remote prefix, iterate ``Path.rglob('*')``, skip
5+
non-files, compute a POSIX-relative remote identifier, call
56
``upload_file`` for each, and collect the successful remote keys. This
67
module factors that walk out so each backend only supplies the two
7-
parts that actually differ — how to assemble the remote identifier
8-
and which per-file upload function to call.
8+
parts that actually differ — how to assemble the remote identifier and
9+
which per-file upload function to call.
910
"""
1011

1112
from __future__ import annotations
1213

1314
from collections.abc import Callable
1415
from pathlib import Path
16+
from typing import NamedTuple
17+
18+
from automation_file.exceptions import DirNotExistsException
19+
20+
21+
class UploadDirResult(NamedTuple):
22+
"""Return value of :func:`walk_and_upload`.
23+
24+
Carries the resolved ``source`` ``Path``, the normalised prefix
25+
(trailing ``/`` stripped), and the list of remote identifiers that
26+
uploaded successfully — so each backend can feed its own log line
27+
without re-doing the Path / prefix work.
28+
"""
29+
30+
source: Path
31+
prefix: str
32+
uploaded: list[str]
1533

1634

1735
def walk_and_upload(
18-
source: Path,
19-
make_remote: Callable[[str], str],
36+
dir_path: str,
37+
prefix: str,
38+
make_remote: Callable[[str, str], str],
2039
upload_one: Callable[[Path, str], bool],
21-
) -> list[str]:
22-
"""Return the list of remote identifiers successfully uploaded from ``source``.
23-
24-
``make_remote`` is called with the POSIX relative path of each file
25-
(no leading slash) and must return the backend-specific remote key.
26-
``upload_one`` receives ``(local_path, remote_key)`` and returns True
27-
on success. Per-file failures are not raised — they are simply
28-
omitted from the returned list, matching the existing backend
40+
) -> UploadDirResult:
41+
"""Walk ``dir_path`` and upload every file via ``upload_one``.
42+
43+
Raises :class:`DirNotExistsException` if ``dir_path`` is not a
44+
directory. ``prefix`` is ``rstrip("/")``-ed before being passed to
45+
``make_remote(normalised_prefix, rel_posix)``, and ``upload_one``
46+
receives ``(local_path, remote_key)`` returning True on success.
47+
Per-file failures are not raised — they are simply omitted from
48+
:attr:`UploadDirResult.uploaded`, matching the existing backend
2949
contract.
3050
"""
51+
source = Path(dir_path)
52+
if not source.is_dir():
53+
raise DirNotExistsException(str(source))
54+
normalised = prefix.rstrip("/")
3155
uploaded: list[str] = []
3256
for entry in source.rglob("*"):
3357
if not entry.is_file():
3458
continue
3559
rel = entry.relative_to(source).as_posix()
36-
remote = make_remote(rel)
60+
remote = make_remote(normalised, rel)
3761
if upload_one(entry, remote):
3862
uploaded.append(remote)
39-
return uploaded
63+
return UploadDirResult(source=source, prefix=normalised, uploaded=uploaded)

automation_file/remote/azure_blob/upload_ops.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from pathlib import Path
66

7-
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
7+
from automation_file.exceptions import FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
99
from automation_file.remote._upload_tree import walk_and_upload
1010
from automation_file.remote.azure_blob.client import azure_blob_instance
@@ -43,20 +43,17 @@ def azure_blob_upload_dir(
4343
name_prefix: str = "",
4444
) -> list[str]:
4545
"""Upload every file under ``dir_path`` to ``container`` under ``name_prefix``."""
46-
source = Path(dir_path)
47-
if not source.is_dir():
48-
raise DirNotExistsException(str(source))
49-
prefix = name_prefix.rstrip("/")
50-
uploaded = walk_and_upload(
51-
source,
52-
lambda rel: f"{prefix}/{rel}" if prefix else rel,
46+
result = walk_and_upload(
47+
dir_path,
48+
name_prefix,
49+
lambda prefix, rel: f"{prefix}/{rel}" if prefix else rel,
5350
lambda local, blob_name: azure_blob_upload_file(str(local), container, blob_name),
5451
)
5552
file_automation_logger.info(
5653
"azure_blob_upload_dir: %s -> %s/%s (%d files)",
57-
source,
54+
result.source,
5855
container,
59-
prefix,
60-
len(uploaded),
56+
result.prefix,
57+
len(result.uploaded),
6158
)
62-
return uploaded
59+
return result.uploaded

automation_file/remote/dropbox_api/upload_ops.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from pathlib import Path
66

7-
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
7+
from automation_file.exceptions import FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
99
from automation_file.remote._upload_tree import walk_and_upload
1010
from automation_file.remote.dropbox_api.client import dropbox_instance
@@ -46,19 +46,16 @@ def dropbox_upload_file(file_path: str, remote_path: str) -> bool:
4646

4747
def dropbox_upload_dir(dir_path: str, remote_prefix: str = "/") -> list[str]:
4848
"""Upload every file under ``dir_path`` to Dropbox under ``remote_prefix``."""
49-
source = Path(dir_path)
50-
if not source.is_dir():
51-
raise DirNotExistsException(str(source))
52-
prefix = remote_prefix.rstrip("/")
53-
uploaded = walk_and_upload(
54-
source,
55-
lambda rel: f"{prefix}/{rel}" if prefix else f"/{rel}",
49+
result = walk_and_upload(
50+
dir_path,
51+
remote_prefix,
52+
lambda prefix, rel: f"{prefix}/{rel}" if prefix else f"/{rel}",
5653
lambda local, remote: dropbox_upload_file(str(local), remote),
5754
)
5855
file_automation_logger.info(
5956
"dropbox_upload_dir: %s -> %s (%d files)",
60-
source,
61-
prefix,
62-
len(uploaded),
57+
result.source,
58+
result.prefix,
59+
len(result.uploaded),
6360
)
64-
return uploaded
61+
return result.uploaded

automation_file/remote/http_download.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def download_file(
7878

7979
written = 0
8080
try:
81-
with open(file_name, "wb") as output, _progress(total_size, file_name) as bar:
81+
with open(file_name, "wb") as output, _progress(total_size, file_name) as progress:
8282
for chunk in response.iter_content(chunk_size=chunk_size):
8383
if not chunk:
8484
continue
@@ -90,7 +90,7 @@ def download_file(
9090
)
9191
return False
9292
output.write(chunk)
93-
bar.update(len(chunk))
93+
progress.update(len(chunk))
9494
except OSError as error:
9595
file_automation_logger.error("download_file write error: %r", error)
9696
return False

automation_file/remote/s3/upload_ops.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from pathlib import Path
66

7-
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
7+
from automation_file.exceptions import FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
99
from automation_file.remote._upload_tree import walk_and_upload
1010
from automation_file.remote.s3.client import s3_instance
@@ -27,20 +27,17 @@ def s3_upload_file(file_path: str, bucket: str, key: str) -> bool:
2727

2828
def s3_upload_dir(dir_path: str, bucket: str, key_prefix: str = "") -> list[str]:
2929
"""Upload every file under ``dir_path`` to ``bucket`` under ``key_prefix``."""
30-
source = Path(dir_path)
31-
if not source.is_dir():
32-
raise DirNotExistsException(str(source))
33-
prefix = key_prefix.rstrip("/")
34-
uploaded = walk_and_upload(
35-
source,
36-
lambda rel: f"{prefix}/{rel}" if prefix else rel,
30+
result = walk_and_upload(
31+
dir_path,
32+
key_prefix,
33+
lambda prefix, rel: f"{prefix}/{rel}" if prefix else rel,
3734
lambda local, key: s3_upload_file(str(local), bucket, key),
3835
)
3936
file_automation_logger.info(
4037
"s3_upload_dir: %s -> s3://%s/%s (%d files)",
41-
source,
38+
result.source,
4239
bucket,
43-
prefix,
44-
len(uploaded),
40+
result.prefix,
41+
len(result.uploaded),
4542
)
46-
return uploaded
43+
return result.uploaded

0 commit comments

Comments
 (0)