-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(config): rename snowflake_username to snowflake_user #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Update environment variable naming for consistency with Snowflake SDK conventions. Changes: - Rename SNOWFLAKE_USERNAME to SNOWFLAKE_USER in service code - Update .env.example with new variable name - Update Helm chart configuration (values.yaml) - Update Helm chart documentation (README.md) - Update technical documentation (snowflake-integration.md) JIRA: LFXV2-692 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>
WalkthroughThe pull request renames the Snowflake service environment variable from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR standardizes the Snowflake username environment variable name from SNOWFLAKE_USERNAME to SNOWFLAKE_USER across the codebase and removes sensitive credential values from documentation examples.
- Renamed environment variable from
SNOWFLAKE_USERNAMEtoSNOWFLAKE_USERfor consistency - Updated all references in code, configuration files, and documentation
- Sanitized example values in documentation by removing actual account credentials
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/lfx-one/src/server/services/snowflake.service.ts | Updated environment variable reference in connection options |
| apps/lfx-one/.env.example | Updated environment variable name in example configuration |
| charts/lfx-v2-ui/values.yaml | Updated Helm chart environment variable configuration |
| charts/lfx-v2-ui/README.md | Updated documentation and parameter table with new variable name |
| docs/architecture/backend/snowflake-integration.md | Updated technical documentation with new variable name and removed sensitive example values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
256-256: Code change is correct and aligns with SDK conventions.The environment variable has been successfully updated from
SNOWFLAKE_USERNAMEtoSNOWFLAKE_USERin the connection options. This aligns with Snowflake SDK naming conventions and is consistent with all configuration and documentation updates across the PR.For a smoother migration path, consider adding temporary fallback logic to support both variable names during the transition period:
const connectionOptions: ConnectionOptions = { account: process.env['SNOWFLAKE_ACCOUNT'] as string, - username: process.env['SNOWFLAKE_USER'] as string, + username: (process.env['SNOWFLAKE_USER'] || process.env['SNOWFLAKE_USERNAME']) as string, role: process.env['SNOWFLAKE_ROLE'] as string,This would allow existing deployments to continue working while they update their configuration. The fallback can be removed in a future release after confirming all environments have migrated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/lfx-one/.env.example(1 hunks)apps/lfx-one/src/server/services/snowflake.service.ts(1 hunks)charts/lfx-v2-ui/README.md(2 hunks)charts/lfx-v2-ui/values.yaml(1 hunks)docs/architecture/backend/snowflake-integration.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/server/services/snowflake.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/server/services/snowflake.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/services/snowflake.service.ts
🔇 Additional comments (4)
charts/lfx-v2-ui/values.yaml (1)
215-215: LGTM! Environment variable rename is consistent.The rename from
SNOWFLAKE_USERNAMEtoSNOWFLAKE_USERaligns with Snowflake SDK conventions. Note that this is a breaking change for existing Helm deployments—ensure all environment-specific values files and deployment configurations are updated accordingly.Verify that deployment teams have been notified of this breaking change and that all environment-specific values files (dev, staging, production) have been updated to use
SNOWFLAKE_USER.charts/lfx-v2-ui/README.md (1)
52-52: Documentation updates look good.Both occurrences of the environment variable have been correctly updated from
SNOWFLAKE_USERNAMEtoSNOWFLAKE_USERin the deployment example and the analytics parameters table, maintaining consistency with the code changes.Also applies to: 184-184
docs/architecture/backend/snowflake-integration.md (1)
88-88: Technical documentation updated consistently.All references to the Snowflake username environment variable have been correctly updated to
SNOWFLAKE_USERacross code examples (line 88) and configuration sections (lines 504, 816), ensuring the documentation remains accurate and aligned with the implementation.Also applies to: 504-504, 816-816
apps/lfx-one/.env.example (1)
39-39: Example environment file updated correctly.The
.env.examplefile now reflects the newSNOWFLAKE_USERenvironment variable, ensuring developers setting up local environments will use the correct variable name from the start.
Summary
Rename the environment variable
SNOWFLAKE_USERNAMEtoSNOWFLAKE_USERacross all configuration files, code, and documentation for consistency with Snowflake SDK conventions.Changes
Files Modified (5 files):
apps/lfx-one/src/server/services/snowflake.service.ts
process.env['SNOWFLAKE_USER']apps/lfx-one/.env.example
charts/lfx-v2-ui/values.yaml
charts/lfx-v2-ui/README.md
docs/architecture/backend/snowflake-integration.md
Verification
✅ No remaining instances of SNOWFLAKE_USERNAME in codebase
✅ All files formatted with yarn format
✅ Build passed successfully
✅ Linting passed
✅ Type checking passed
Rationale
The
SNOWFLAKE_USERnaming convention is more consistent with Snowflake SDK standards and common practice in Snowflake integrations.JIRA
LFXV2-692
Generated with Claude Code