--keepalive-ticks CLI option and DSN parameter

[rolandwalker]

Description

This allows the user to set keepalives on a per-connection basis. The motivation is a persisted DSN, but it seems inconsistent not to include the CLI flag.

Checklist

• I added this contribution to the changelog.md file.
• I added my name to the AUTHORS file (or it's already there).
• To lint and format the code, I ran

  uv run ruff check && uv run ruff format && uv run mypy --install-types .

[github-actions]

Findings

1. Regression: keepalive config ignored outside CLI path.
   MyCli.connect() now unconditionally sets self.keepalive_ticks from the keepalive_ticks argument defaulting to 0, which disables keepalives even when default_keepalive_ticks is set in config. Callers that don’t pass the new param (notably the IPython magic path) will now silently lose keepalive behavior.
   Fix: make keepalive_ticks default to None in connect() and fall back to self.default_keepalive_ticks when it’s None, or set it to self.default_keepalive_ticks before assigning.
   File: mycli/main.py:528 and mycli/main.py:577
   Also affects: mycli/magic.py:33 (caller not passing keepalive).

2. Unintended stdout noise during keepalive.
   keepalive_hook() prints -------> keepalive ping on every ping. This is user-visible output that will pollute the prompt, logs, and scripted output. Looks like a stray debug line.
   Fix: remove the print() or gate it behind a debug flag/logger.
   File: mycli/main.py:1016

Missing Tests / Edge Cases

1. Add a test that default_keepalive_ticks from config is honored when connect() is invoked without CLI/DSN (e.g., IPython magic path). This would catch the regression above.
   Targets: mycli/magic.py call path or a direct MyCli.connect() without keepalive_ticks.

2. Add a test for CLI precedence: CLI flag should override DSN param, and both should override config default. You added a DSN param test, but not the CLI-flag override behavior.

Notes
No security issues found.

[github-actions]

Findings

1. Regression: keepalive_ticks can become None and crash keepalive hook

   • In mycli/main.py, MyCli.connect() now unconditionally sets self.keepalive_ticks = keepalive_ticks. Several callers invoke connect() without the new argument (e.g., mycli/magic.py, test/test_tabular_output.py). This makes self.keepalive_ticks None, and later comparisons like self.keepalive_ticks < 1 or >= 1 will raise TypeError.
   • Fix: default to self.default_keepalive_ticks when keepalive_ticks is None.
   • Files: mycli/main.py, mycli/magic.py, test/test_tabular_output.py

2. Input validation gap for DSN keepalive_ticks

   • int(params[0]) is used directly for the DSN query param. If the user passes a non-integer (keepalive_ticks=abc), this will raise ValueError and terminate.
   • Consider guarding with a try/except and emitting a click.BadParameter (or similar) for a clean CLI error.
   • File: mycli/main.py

Missing tests / edge cases

1. CLI flag precedence and propagation

   • Add a test to ensure --keepalive-ticks is passed into connect() (and overrides DSN) similar to the DSN test added.
   • File: test/test_main.py

2. Default behavior when connect() is called without keepalive_ticks

   • Add a regression test that calls connect() without the arg (e.g., in test/test_tabular_output.py or a unit test) and ensures keepalive remains enabled/disabled based on default_keepalive_ticks.
   • File: test/test_tabular_output.py or test/test_main.py

Suggested fix (summary)

• Update MyCli.connect() to set:
  • self.keepalive_ticks = keepalive_ticks if keepalive_ticks is not None else self.default_keepalive_ticks
• Add tests for CLI flag and default behavior.

If you want, I can draft the patch and tests.

[github-actions]

Findings

1. Regression: keepalive defaults ignored outside CLI path
   mycli/main.py:577 assigns self.keepalive_ticks = keepalive_ticks directly. When MyCli.connect() is called from non-CLI code paths (e.g., IPython magic in mycli/magic.py), keepalive_ticks is None, so keepalives are disabled even if default_keepalive_ticks is set in config. Previously the keepalive hook used self.default_keepalive_ticks, so defaults applied everywhere.
   Action: In MyCli.connect(), set self.keepalive_ticks = keepalive_ticks if keepalive_ticks is not None else self.default_keepalive_ticks to preserve config defaults for direct callers.

2. Robustness: DSN parsing can crash on invalid keepalive_ticks
   mycli/main.py:2007-2009 casts int(params[0]) without validation. A DSN like ...?keepalive_ticks=abc or empty value raises ValueError and aborts the CLI. Other DSN params are strings/booleans, so this is a new hard failure.
   Action: Wrap the int() in try/except and emit a user-facing error (click.secho(..., err=True) + sys.exit(1)), or ignore invalid values with a warning.

Missing tests / edge cases

1. CLI flag behavior isn’t covered. Add a test that --keepalive-ticks 30 passes through to connect_args["keepalive_ticks"].
2. Add a test for invalid DSN keepalive_ticks (non-integer) to ensure a clean error path rather than a traceback.
3. Add a test to ensure MyCli.connect() (non-CLI) honors default_keepalive_ticks when keepalive_ticks is None. This guards the regression above.

If you want, I can draft the exact test cases.