Skip to content

test_nos_server_config_1 crashes with a raw ValueError when SFTP_* env vars are unset, instead of failing clearly #216

Description

@dk107dk

tests/util/test_util_nos.py::TestUtilNos::test_nos_server_config_1 fails with an unhandled ValueError when the SFTP_* environment variables (SFTP_SERVER, SFTP_PORT, etc.) are not set -- e.g. in a dev environment where ~/dev/exports.sh has not been sourced.

Traceback (relevant frames):

csvpath/util/var_utility.py -- VarUtility.parse_var_value() / value_or_var_value():
if the config value is an all-caps token (e.g. "SFTP_PORT") and os.getenv() for it
returns None, the literal token string is returned unchanged, by design (this is
documented behavior, not itself a bug).

tests/util/test_util_nos.py:18-23 then builds:
    path = f"sftp://{server}:{port}/nos_test.txt"
with server/port still literally "SFTP_SERVER"/"SFTP_PORT", and passes it to
DataFileWriter.

.venv/.../smart_open/ssh.py:115 parse_uri() -> urllib.parse.urlsplit(uri).port
.venv/.../urllib/parse.py:182:
    raise ValueError(f"Port could not be cast to integer value as {port!r}")
ValueError: Port could not be cast to integer value as 'SFTP_PORT'

So an unsubstituted placeholder ("SFTP_PORT" the string, not a real port number) propagates all the way from config into a third-party library's URL parser several layers down, where it blows up with a confusing raw ValueError instead of failing clearly and close to the source.

David: reproduces in a dev VM that lacks the real SFTP_* env vars (exports.sh not sourced there); passes fine on the host where those are exported. So the underlying cause is an expected environment gap, not a functional bug in the port-vs-None fix from PR #215. Still, the failure mode itself is bad -- it should not be possible for an unresolved ALL_CAPS placeholder to reach a third-party URL parser and crash there. Worth either:

  • having the SFTP-dependent tests skip cleanly when the real SFTP_* env vars are not present, rather than attempt a live connection with placeholder values, or
  • having VarUtility/Config raise a clear, early, csvpath-level error when a required env-var-backed config value is still an unresolved placeholder at the point of use.

Found while re-verifying the full suite after merging PR #215 (SFTP/Nos port fix) and PR #214 (references-v3 function registry) together -- this failure is part of the long-standing "known 11-failure" local-environment baseline, unchanged by either of those PRs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions