-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(env): add and document env-var ADMIN_API_KEY_FILE #255
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #255 +/- ##
===========================================
- Coverage 70.87% 70.84% -0.03%
===========================================
Files 35 35
Lines 8967 8979 +12
Branches 523 526 +3
===========================================
+ Hits 6355 6361 +6
- Misses 2610 2616 +6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
error @permaweb/aoconnect@0.0.57: The engine "yarn" is incompatible with this module. Expected version "please-use-npm". Got "1.22.22" 📝 WalkthroughWalkthroughThis pull request introduces a new method for configuring the admin API key by adding support for specifying the key through a file path. The changes span across three files: Changes
Sequence DiagramsequenceDiagram
participant Config as Configuration
participant EnvFile as .env File
participant KeyFile as Admin Key File
Config->>EnvFile: Check ADMIN_API_KEY_FILE
alt File path specified
Config->>KeyFile: Read file contents
alt File exists
KeyFile-->>Config: Return key contents
Config->>Config: Update ADMIN_API_KEY
else File not found
Config->>Config: Throw error
end
else No file path
Config->>EnvFile: Use ADMIN_API_KEY directly
end
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (2)
src/config.ts (2)
20-20
: Consider using asynchronous file operations
While usingreadFileSync
andexistsSync
is acceptable for a one-time startup configuration, switching to their asynchronous counterparts may allow better scalability for future expansions or if any additional file I/O is introduced.
39-49
: Validate file reading logic and error handling
The logic for reading the admin API key from a file is clear. However:
- Ensure exceptions from
readFileSync
are properly handled (e.g., permission errors).- Consider logging a warning or additional diagnostic information if the file exists but cannot be read.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example
(1 hunks)docs/envs.md
(1 hunks)src/config.ts
(2 hunks)
🔇 Additional comments (4)
src/config.ts (1)
35-35
: Confirm necessity of mutableADMIN_API_KEY
Switching fromconst
tolet
allows reassigning the variable, but consider potential concurrency or security implications if this variable is accessed or mutated in other parts of the code..env.example (2)
34-34
: Good clarification on the admin API key comment
This comment clarifies thatADMIN_API_KEY
is the key used for accessing admin API.
37-40
:⚠️ Potential issueFix the “precedene” typo
There is a minor spelling error: “precedene” → “precedence”.-# it takes precedene over ADMIN_API_KEY +# it takes precedence over ADMIN_API_KEYLikely invalid or redundant comment.
docs/envs.md (1)
20-20
: Excellent addition ofADMIN_API_KEY_FILE
Clearly documents how to set the admin API key via file. Just ensure the spelling of “precedence” is consistent across files.
No description provided.