feat(config): allow setup_credentials() to configure only a subset of databases #192
Conversation
WalkthroughThe setup_credentials function in src/pystatis/config.py now accepts db_names and a validate flag, selects target databases accordingly, validates provided names, ensures config sections exist per target, conditionally validates credentials, writes them, saves the config, and logs the specific databases updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as setup_credentials(db_names, validate)
participant CFG as Config Store
participant VAL as Validator
participant LOG as Logger
User->>CLI: Invoke with db_names (or None), validate flag
CLI->>CLI: Determine targets (all if None, else provided)
CLI->>CLI: Check targets ∈ supported
alt Unknown target(s)
CLI->>User: Raise PystatisConfigError (unknown + supported)
else Valid targets
loop For each target DB
CLI->>CFG: Ensure section exists (create if missing)
CLI->>User: Prompt username/password
alt validate == True
CLI->>VAL: Validate credentials for target
VAL-->>CLI: Validation result
alt Validation OK
CLI->>CFG: Write credentials
else Validation fails
CLI-->>User: Propagate/handle failure
end
else validate == False
CLI->>CFG: Write credentials (skip validation)
end
end
CLI->>CFG: Save config file
CLI->>LOG: Log updated databases and config path
end
note over CLI,LOG: Log message now lists specific databases updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pystatis/config.py (1)
191-201: Do not echo or print secrets; use getpass for passwordscurrent behavior prints credentials to stdout (“You entered: …”), which is a security/PII leak. For passwords, also avoid echoed input. Use getpass and remove the echo entirely.
Apply this diff:
-def _get_user_input(db: str, field: str) -> str: - """Get user input for the given database and field.""" - env_var = os.environ.get(f"PYSTATIS_{db.upper()}_API_{field.upper()}") - - if env_var is not None: - return env_var - - user_input = input(f"Please enter your {field} for the database {db}:") - print("You entered: " + user_input) - - return user_input +def _get_user_input(db: str, field: str) -> str: + """Get user input for the given database and field.""" + env_var = os.environ.get(f"PYSTATIS_{db.upper()}_API_{field.upper()}") + if env_var is not None: + return env_var + prompt = f"Please enter your {field} for the database {db}: " + if field.lower() == "password": + # Do not echo passwords to stdout. + return getpass.getpass(prompt) + return input(prompt)Add this import at the top-level imports:
import getpass
🧹 Nitpick comments (3)
src/pystatis/config.py (3)
154-158: Make subset handling more robust: deduplicate and handle empty input gracefullyThe core logic looks good. Two small robustness tweaks:
- Deduplicate explicit db_names while preserving order to avoid double prompts/writes.
- If an empty list is passed (db_names=[]), treat it as a no-op and return early with an info log instead of writing the config with no changes.
Also consider accepting a broader type (e.g., Sequence[str]) in the future for flexibility, but that's optional.
Apply this diff:
- supported = get_supported_db() - targets = supported if db_names is None else db_names + supported = get_supported_db() + # Preserve order and drop duplicates when explicit targets are given + targets_raw = supported if db_names is None else db_names + targets = list(dict.fromkeys(targets_raw)) + if not targets: + logger.info("No databases provided; skipping credential setup. Path: %s.", _build_config_file_path()) + return
155-155: Docstring: document new parameters and failure modesDocument db_names semantics (None vs list) and validate behavior to make the intent crystal clear.
Apply this diff:
- """Setup credentials for one or more supported databases.""" + """Setup credentials for one or more supported databases. + + Args: + db_names: List of database names to configure. If None (default), all supported databases are targeted. + validate: Whether to validate the provided credentials against the remote service (default: True). + + Raises: + PystatisConfigError: If unknown database names are provided or validation fails. + """
172-175: Harden validation against network/HTTP errorsIf db.check_credentials_are_valid raises (e.g., network issues), it will bubble up unhandled. Wrap to surface a friendly PystatisConfigError and suggest validate=False as a fallback.
Apply this diff:
- if validate and not db.check_credentials_are_valid(db_name): - raise PystatisConfigError( - f"Provided credentials for database '{db_name}' are not valid! Please provide the correct credentials." - ) + if validate: + try: + is_valid = db.check_credentials_are_valid(db_name) + except Exception as e: + raise PystatisConfigError( + f"Could not validate credentials for database '{db_name}': {e}. " + "You can call setup_credentials(..., validate=False) to skip validation." + ) from e + if not is_valid: + raise PystatisConfigError( + f"Provided credentials for database '{db_name}' are not valid! Please provide the correct credentials." + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/pystatis/config.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/pystatis/config.py (2)
src/pystatis/exception.py (1)
PystatisConfigError(10-13)src/pystatis/db.py (1)
check_credentials_are_valid(107-129)
🔇 Additional comments (4)
src/pystatis/config.py (4)
159-164: Good validation: clear error for unknown target namesNice guardrail and actionable error message listing supported databases.
165-168: Good: ensure section exists before writingCreating the section if missing avoids KeyError paths and keeps config consistent.
179-183: LGTM: specific and useful loggingReporting the exact databases updated and the config path is helpful.
154-154: setup_credentials signature change verified
All call sites (tests/test_config.py:79and references insrc/pystatis/db.py) use the default parameter only—no calls pass a second positional argument. The new*, validate: bool = Truekeyword-only signature is backward compatible.
|
Just a quick comment, I did not write this function to enforce all three, you are prompted for all three but you can just leave the information blank or empty so you do not need to have three pairs of credentials. But anyway good idea and I will try to review soon! |
2 participants
Summary
db_names: list[str] | None = Noneparameter (andvalidate: bool = True) tosetup_credentials().db_namesis provided (e.g.,["genesis"]), only those databases are prompted/read from env and (optionally) validated.setup_credentials()with no arguments still configures all supported databases.config.iniuntouched.Motivation / Problem
Backward compatibility
setup_credentials()continues to handle all databases.Usage examples
Environment variables (unchanged)
PYSTATIS_GENESIS_API_USERNAME/PYSTATIS_GENESIS_API_PASSWORDPYSTATIS_ZENSUS_API_USERNAME/PYSTATIS_ZENSUS_API_PASSWORDPYSTATIS_REGIO_API_USERNAME/PYSTATIS_REGIO_API_PASSWORDDocs updates
Add:
setup_credentials(["genesis"]).”Summary by CodeRabbit