feat: Add modern DSS transfer utilities - #242
Conversation
krowvin
changed the title
krowvin
left a comment
krowvin
left a comment
There was a problem hiding this comment.
Few thoughts for the reviewer
| def dss2cwms_main() -> None: | ||
| _enable_terminal_colors() | ||
| import_cmd.main(args=_normalize_legacy_args(sys.argv[1:]), prog_name="dss2cwms") | ||
|
|
||
|
|
||
| def cwms2dss_main() -> None: | ||
| _enable_terminal_colors() | ||
| export_cmd.main(args=_normalize_legacy_args(sys.argv[1:]), prog_name="cwms2dss") |
There was a problem hiding this comment.
Didn't want to sneak these in.
But this would create commands in the path for cwms2dss ...etc or dss2cwms ...etc in an attempt to make for backwards compatibility.
We don't do this for other commands and this could be starting something.
Should we just stick with cwms-cli dss import/export or is it OK to keep these two commands as well?
| def _normalize_legacy_args(args: list[str]) -> list[str]: | ||
| """Translate legacy ``-option=value`` forms into Click-compatible tokens.""" | ||
| normalized = [] | ||
| prefixes = ("-dss", "-db", "-id", "-f2", "-tz", "-o", "-f", "-p", "-v", "-l") |
There was a problem hiding this comment.
Instead of translating old args should we instead throw an error and ask for the new args to be used?
| _DSS_INTERVAL = { | ||
| "minute": "MIN", | ||
| "hour": "HOUR", | ||
| "day": "DAY", | ||
| "week": "WEEK", | ||
| "month": "MON", | ||
| "year": "YEAR", | ||
| } |
There was a problem hiding this comment.
I couldn't find these exposed in hecdss, but would prefer to get it from there in case DECADE, etc was every added.
| for line_number, row in _rows(filename): | ||
| if len(row) != 4: | ||
| raise MappingError( | ||
| f"{filename}:{line_number}: expected pathname,tsid,CWMS_unit,factor" |
There was a problem hiding this comment.
Wondering if I should link to the docs here too
| pathname, tsid, cwms_unit, factor_text = row | ||
| normalized = normalize_pathname(pathname) | ||
| if len(tsid.split(".")) != 6: | ||
| raise MappingError(f"{filename}:{line_number}: invalid CWMS TSID: {tsid}") |
There was a problem hiding this comment.
Or really any mapping error link to the docs
| dss2cwms = "cwmscli.dss.compat:dss2cwms_main" | ||
| cwms2dss = "cwmscli.dss.compat:cwms2dss_main" |
There was a problem hiding this comment.
These get exposed along with the cwms-cli command, related to my comment above about how we don't expose commands like this for any other commands but perhaps this is a one-off because these commands existed as stand alones prior.
1 participant
Summary
cwms-cli dss importandcwms-cli dss exportwith legacy entry pointsCloses #94
Validation
poetry run pytest(261 passed)poetry run sphinx-build -W -b html docs docs/_build/html