-
-
Notifications
You must be signed in to change notification settings - Fork 52
[Examples] add AWS credentials environment validator #307
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
base: main
Are you sure you want to change the base?
Conversation
- Created `EnvValidator` class in `src/EnvValidator.php` to centralize AWS credentials environment variable validation - Updated `examples/bedrock/` to use the new validator
3b229e7
to
c524bd8
Compare
@chr-hertel Can you review this ? |
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.
We usually haven env() shortcut in other examples, I would prefer to use this
@OskarStark you mean to env() instead of global var thingy |
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.
The difference with these examples is that env(...)
is usually used to access the $_SERVER
and throw an error if not set. In our Bedrock bridge we don't have to do that since the aws lib we're using is fetching the env vars itself.
Again, like with #313, I'm not convinced to have an extra class here. I see the point that this looks a bit odd right now, but unless we can come up with a better idea than an extra class, i'd rather keep it like that.
EnvValidator
to centralize AWS credentials environment variable validationexamples/bedrock/
to use the new validator