Skip to content
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

POC: Add environment variable injection to plugin #77

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryanolee
Copy link

@ryanolee ryanolee commented Sep 29, 2021

What?

This a proof of concept for the proposal given in #76

Why?

To show the viability of implementing such a change.

How?

Currently this PR parses the new configuration option and adds environment variables to the handler associated with the file. There is no duplication detection / function specific isolation but this should be possible by copying the handlers js file and later on updating the handler config to use the file with the isolated ENV over the shared handler file.

What needs doing still:

In the event this is seen to be a reasonable feature to implement proper environment separation / collection will need to be set up. Beyond that documentation will need to be written preferably with a set of unit tests. If possible support should also be added for python runtimes using the os.environ over the process.env variable.

Beyond that a few tests need writing for the implementation.

In theory we could use something like https://webpack.js.org/plugins/environment-plugin/ over injecting the env vars ourselves 🤔

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.1%) to 13.187% when pulling 7b046e8 on ryanolee:feature/InjectEnv into 157f0af on silvermine:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.1%) to 13.187% when pulling 7b046e8 on ryanolee:feature/InjectEnv into 157f0af on silvermine:master.

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