On Fri, Jul 25, 2025 at 11:15 AM Luca Vizzarro wrote: > > + > +@overload > +def make_file_path(node: Node, file_name: str, custom_path: PurePath | > None = None) -> PurePath: ... > + > + > +@overload > +def make_file_path(node: None, file_name: str, custom_path: PurePath | > None = None) -> Path: ... > + > + > +def make_file_path( > + node: Node | None, file_name: str, custom_path: PurePath | None = None > Maybe it makes sense to set a default value of None for node? That way people don't have to pass in None every time they want to make a path on the DTS engine system. > > + def open( > + self, file_mode: BinaryMode | TextMode = "rb", buffering: int = -1 > + ) -> Union["ArtifactFile", TextIOWrapper]: > + """Open the artifact file. > + > + Args: > + file_mode: The mode of file opening. > + buffering: The size of the buffer to use. If -1, the default > buffer size is used. > + > + Returns: > + An instance of :class:`ArtifactFile` or > :class:`TextIOWrapper`. > + """ > + if self._fd is not None and not self._fd.closed: > + self._logger.warning( > + f"Artifact {self.path} is already open. Closing the > previous file descriptor." > + ) > + self._fd.close() > + elif not self._directories_created: > + self.mkdir() > + > + # SFTPFile does not support text mode, therefore everything needs > to be handled as binary. > + if "t" in file_mode: > + actual_mode = cast(BinaryMode, cast(str, > file_mode).replace("t", "") + "b") > Is it worth logging this event to prevent confusion? (where we change the requested mode to binary mode) + elif "b" not in file_mode: > + actual_mode = cast(BinaryMode, file_mode + "b") > same > > -- > 2.43.0 > > Reviewed-by: Patrick Robb