-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
chore(functions/concepts): split functions into subdirs #8555
Conversation
Here is the summary of changes. You are about to add 6 region tags.
This comment is generated by snippet-bot.
|
# limitations under the License. | ||
|
||
# [START functions_concepts_requests] | ||
def make_request(request): |
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.
This is the same sample as connection_pooling in the tips. Can we deduplicate?
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.
They're slightly different - see these two lines.
(That one uses connection pooling, while this one doesn't.)
instance_var = heavy_computation() | ||
|
||
|
||
def scope_demo(request): |
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.
Duplicated from tips PR
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.
I assume you're referring to this sample?
They're similar, but not identical - that sample caches the "local" variable globally, while this sample does not.
There's also this Cloud Run sample, which may be duplicative:
we could reuse [this] one, but we'd have to split them up later on if differing requirements emerge.
IOW splitting up = future-proofing; keeping them together = optimizing for "right now"
f82fa18
to
35dbd73
Compare
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.
LGTM
No description provided.