-
Notifications
You must be signed in to change notification settings - Fork 42
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
test: do GCF cleanup in both presubmit and e2e tests #423
Merged
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
bced046
test: do GCF cleanup in both presubmit and e2e tests
shobsi b44fa1d
use functions client from session
shobsi 16da671
Merge remote-tracking branch 'refs/remotes/github/main' into shobs-gc…
shobsi ff0787a
Merge remote-tracking branch 'refs/remotes/github/main' into shobs-gc…
shobsi 63f690d
Merge branch 'main' into shobs-gcf-cleanup
tswast 0d5cd23
Merge remote-tracking branch 'refs/remotes/github/main' into shobs-gc…
shobsi 2fe59eb
address review comments
shobsi 42a52c1
Merge remote-tracking branch 'github/shobs-gcf-cleanup' into shobs-gc…
shobsi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
import typing | ||
from typing import Dict, Optional | ||
|
||
from google.api_core.exceptions import NotFound, ResourceExhausted | ||
import google.cloud.bigquery as bigquery | ||
import google.cloud.bigquery_connection_v1 as bigquery_connection_v1 | ||
import google.cloud.exceptions | ||
|
@@ -34,7 +35,20 @@ | |
import test_utils.prefixer | ||
|
||
import bigframes | ||
from tests.system.utils import convert_pandas_dtypes | ||
from tests.system.utils import ( | ||
convert_pandas_dtypes, | ||
delete_cloud_function, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Import the "utils" package/module, not the individual functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, thanks. |
||
get_cloud_functions, | ||
get_remote_function_endpoints, | ||
) | ||
|
||
# Use this to control the number of cloud functions being deleted in a single | ||
# test session. This should help soften the spike of the number of mutations per | ||
# minute tracked against a quota limit (default 60, increased to 120 for | ||
# bigframes-dev project) by the Cloud Functions API | ||
# We are running pytest with "-n 20". Let's say each session lasts about a | ||
# minute, so we are setting a limit of 120/20 = 6 deletions per session. | ||
MAX_NUM_FUNCTIONS_TO_DELETE_PER_SESSION = 6 | ||
|
||
CURRENT_DIR = pathlib.Path(__file__).parent | ||
DATA_DIR = CURRENT_DIR.parent / "data" | ||
|
@@ -1040,3 +1054,53 @@ def floats_bf(session, floats_pd): | |
@pytest.fixture() | ||
def floats_product_bf(session, floats_product_pd): | ||
return session.read_pandas(floats_product_pd) | ||
|
||
|
||
@pytest.fixture(scope="session", autouse=True) | ||
def cleanup_cloud_functions(session, cloudfunctions_client, dataset_id_permanent): | ||
"""Clean up stale cloud functions.""" | ||
permanent_endpoints = get_remote_function_endpoints( | ||
session.bqclient, dataset_id_permanent | ||
) | ||
delete_count = 0 | ||
for cloud_function in get_cloud_functions( | ||
cloudfunctions_client, | ||
session.bqclient.project, | ||
session.bqclient.location, | ||
shobsi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name_prefix="bigframes-", | ||
): | ||
# Ignore bigframes cloud functions referred by the remote functions in | ||
# the permanent dataset | ||
if cloud_function.service_config.uri in permanent_endpoints: | ||
continue | ||
|
||
# Ignore the functions less than one day old | ||
age = datetime.now() - datetime.fromtimestamp( | ||
cloud_function.update_time.timestamp() | ||
) | ||
if age.days <= 0: | ||
continue | ||
|
||
# Go ahead and delete | ||
try: | ||
delete_cloud_function(cloudfunctions_client, cloud_function.name) | ||
delete_count += 1 | ||
if delete_count >= MAX_NUM_FUNCTIONS_TO_DELETE_PER_SESSION: | ||
break | ||
except NotFound: | ||
# This can happen when multiple pytest sessions are running in | ||
# parallel. Two or more sessions may discover the same cloud | ||
# function, but only one of them would be able to delete it | ||
# successfully, while the other instance will run into this | ||
# exception. Ignore this exception. | ||
pass | ||
except ResourceExhausted: | ||
# This can happen if we are hitting GCP limits, e.g. | ||
# google.api_core.exceptions.ResourceExhausted: 429 Quota exceeded | ||
# for quota metric 'Per project mutation requests' and limit | ||
# 'Per project mutation requests per minute per region' of service | ||
# 'cloudfunctions.googleapis.com' for consumer | ||
# 'project_number:1084210331973'. | ||
# [reason: "RATE_LIMIT_EXCEEDED" domain: "googleapis.com" ... | ||
# Let's stop further clean up and leave it to later. | ||
break |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Nit: Use import statements for packages and modules only, not for individual types, classes, or functions.
https://google.github.io/styleguide/pyguide.html#22-imports
The only exception is with the typing package.
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.
Done, thanks.