Skip to content

refactor(clp-package)!: Update user in credentials config file for consistency (resolves #1609):#1610

Merged
junhaoliao merged 2 commits into
y-scope:mainfrom
junhaoliao:credentials-key-value-consistency
Nov 17, 2025
Merged

refactor(clp-package)!: Update user in credentials config file for consistency (resolves #1609):#1610
junhaoliao merged 2 commits into
y-scope:mainfrom
junhaoliao:credentials-key-value-consistency

Conversation

@junhaoliao

@junhaoliao junhaoliao commented Nov 16, 2025

Copy link
Copy Markdown
Member
  • Rename user to username.
  • Update default user to clp-user in template credentials.

Description

This PR introduces a BREAKING change: database and redis usernames must now be configured under the username fields in etc/credentials.yml within the CLP package. e.g.,

etc/credentials.yml:

# Database credentials
database:
  # OLD
  # user: "user"
  # NEW
  username: "clp-user"
  ...

# Queue credentials
queue:
  # OLD
  # user: "user"
  # NEW
  username: "clp-user"
  ...

...

The PR fixes inconsistency where credential key names and default values differed between the template file, Pydantic config objects, and auto-generated credentials. All components now consistently use username as the key name with clp-user as the default value.

Changes:

  • Updated credentials.template.yml to show username: "clp-user" (instead of user: "user")
  • Modified clp_config.py to read database.username and queue.username from YAML
  • Updated general.py to generate credentials with username keys and clp-user as default
  • Fixed Presto deployment script to read database.username and renamed variable to database_username

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Verified all credential loading code now reads username keys
  • Confirmed template default values (clp-user) match auto-generated credentials
  • Checked documentation (already showed correct username: "clp-user" format)
  • Searched codebase for remaining references to old user key format
task
cd build/clp-package
./sbin/start-clp.sh
./sbin/compress.sh ~/samples/postgresql.jsonl
# observed no error

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Updated credential key naming from "user" to "username" for database and queue components to ensure consistent credential handling across the system. Configuration templates and credential loading have been updated accordingly.

…of `user`; Update values in the credentials template (fixes y-scope#1609).
@junhaoliao junhaoliao requested a review from a team as a code owner November 16, 2025 04:10
@coderabbitai

coderabbitai Bot commented Nov 16, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A coordinated refactoring standardizes credential field naming from "user" to "username" across credential generation, configuration loading, template definitions, and deployment scripts for database and queue components, while Redis credentials remain unchanged.

Changes

Cohort / File(s) Summary
Credential generation and template
components/clp-package-utils/clp_package_utils/general.py, components/package-template/src/etc/credentials.template.yml
Database and queue credential entries updated to use "username" key instead of "user"; template example value changed from "user" to "clp-user"
Configuration and deployment reading
components/clp-py-utils/clp_py_utils/clp_config.py, tools/deployment/presto-clp/scripts/init.py
Credential loading paths updated to read from ".username" instead of ".user"; local variable renamed from database_user to database_username in deployment script

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify consistency across all four files that the refactoring is uniformly applied
  • Confirm Redis credentials were intentionally left unchanged (not missed)
  • Check that environment variable assignments still reference the correctly-renamed variables in init.py

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the primary change: standardizing credential keys from 'user' to 'username' across configuration files and code for consistency.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kirkrodrigues

Copy link
Copy Markdown
Member

Isn't this a breaking change?

@junhaoliao junhaoliao changed the title fix(clp-package): Standardize credential keys to usernames instead of user; Update values in the credentials template (fixes #1609). fix(clp-package)!: Standardize credential keys to usernames instead of user; Update values in the credentials template (fixes #1609). Nov 17, 2025
@junhaoliao junhaoliao requested review from kirkrodrigues and removed request for kirkrodrigues November 17, 2025 22:01
@junhaoliao junhaoliao changed the title fix(clp-package)!: Standardize credential keys to usernames instead of user; Update values in the credentials template (fixes #1609). refactor(clp-package)!: Standardize credential keys to usernames instead of user; Update values in the credentials template (resolves #1609). Nov 17, 2025

@kirkrodrigues kirkrodrigues left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the PR title & body, how about:

refactor(clp-package)!: Update `user` in credentials config file for consistency: (resolves #1609)
- Rename `user` to `username`.
- Update default user to `clp-user` in template credentials.

@junhaoliao junhaoliao changed the title refactor(clp-package)!: Standardize credential keys to usernames instead of user; Update values in the credentials template (resolves #1609). refactor(clp-package)!: Update user in credentials config file for consistency (resolves #1609): Nov 17, 2025
@junhaoliao junhaoliao merged commit f0fa3ee into y-scope:main Nov 17, 2025
29 checks passed
@junhaoliao junhaoliao deleted the credentials-key-value-consistency branch November 17, 2025 22:18
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
…consistency (resolves y-scope#1609): (y-scope#1610)

- Rename `user` to `username`.
- Update default user to `clp-user` in template credentials.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants