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

Add legacy GCF Python 3.7 behavior #77

Merged
merged 3 commits into from
Aug 17, 2020
Merged

Add legacy GCF Python 3.7 behavior #77

merged 3 commits into from
Aug 17, 2020

Conversation

asriniva
Copy link
Contributor

No description provided.

@google-cla google-cla bot added the cla: yes label Aug 11, 2020
tests/test_functions.py Outdated Show resolved Hide resolved
tests/test_functions.py Outdated Show resolved Hide resolved
tests/test_functions.py Outdated Show resolved Hide resolved
tests/test_functions.py Outdated Show resolved Hide resolved
tests/test_functions/returns_none/main.py Outdated Show resolved Hide resolved
Comment on lines +179 to +180
os.environ["FUNCTION_TRIGGER_TYPE"] = signature_type
os.environ["FUNCTION_NAME"] = os.environ.get("K_SERVICE", target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this raises some questions about how the environment variables should be handled in the 3.7 buildpack runtime. This has the effect of setting some of them, which is good... but why these? Should we be setting all of them?

Furthermore, should we set FUNCTION_TRIGGER_TYPE and FUNCTION_NAME for 3.8 as well? It would make it easier for users to migrate from 3.7 to 3.8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remaining env vars don't have an equivalent in newer GCF runtimes, so they have to be set outside of the functions framework/buildpack.

For the second question, there was precedent in the nodejs8 -> nodejs10 transition, where the older env vars were dropped. To prevent carrying on the tech debt I think they should not be available in Python 3.8.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take this discussion elsewhere 🙂

@di di merged commit 81b1c32 into GoogleCloudPlatform:version-1.x-dev Aug 17, 2020
di pushed a commit that referenced this pull request Aug 19, 2020
* Add legacy GCF Python 3.7 behavior

* Add test

* Modify tests
di added a commit that referenced this pull request Aug 19, 2020
* Version 1.5.0 (#69)

* Revert "Version 2.0.0 (#67)"

This reverts commit f2471b4.

* Revert "Add Cloud Events support for #55 (#56) (#64)"

This reverts commit 8f3fe35.

* Version 1.5.0

* Add legacy GCF Python 3.7 behavior (#77)

* Add legacy GCF Python 3.7 behavior

* Add test

* Modify tests

* Version 1.6.0 (#81)

Co-authored-by: Arjun Srinivasan <69502+asriniva@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants