-
Notifications
You must be signed in to change notification settings - Fork 579
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: don't require .env to exist #427
Conversation
// Load the .env file | ||
const result = config({ path: envPath }); | ||
// attempt to Load the .env file into process.env | ||
const result = config(envPath ? { path: envPath } : {}); |
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.
Confirming that config({}) doesn't cause any errors?
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.
It does not, I checked
FYI: There is another approach to agent specific envs added in: #410 still via env but in a segmented way. |
I saw that, complementary to this. Wasn't sure if that's going to get merged so left it out of this description |
Awesome - re comments. Yeah totally makes sense. Feel free to add a quick
note in the README about .env files being optional. I know it may cause
merge conflicts between these 2 PRs, but it does seem important to have!
Thanks for doing this :)
…On Tue, Nov 19, 2024 at 7:02 PM Odilitime ***@***.***> wrote:
I saw that, complementary to this. Wasn't sure if that's going to get
merged so left it out of this description
—
Reply to this email directly, view it on GitHub
<#427 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADS6ROIFKY3GDMIXH2FIZPT2BP3S5AVCNFSM6AAAAABSDQRNZSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBXGI2DKMJZHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Looks like the limitations are: feat|fix|docs|style|refactor|test|chore:any description see
|
LGTM! 🥇 |
Risks
Medium, haven't discussed this widely
Background
What does this PR do?
Removes the requirement for .env to exists
What kind of change is this?
Improvement
Why are we doing this? Any context or related work?
Large multiagent instances will have to rely on character.json to have all the settings and secrets for that agent. Have a default .env in these instances to set defaults might be helpful but also hurtful. This allows the admin to choose if they want any defaults across all their agents or nots.
Documentation changes needed?
My changes do not require a change to the project documentation.
Testing
Ran against stable-11-17 branch without issue