-
Notifications
You must be signed in to change notification settings - Fork 58
feat: warn if default cloud_function_service_account is used #1424
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
8fed967
feat: warn if default cloud_function_service_account is used
arwas11 146ef07
Merge remote-tracking branch 'origin/main' into b398853216-service-ac…
arwas11 819dba7
fix docstring style
arwas11 afee1aa
Merge branch 'main' into b398853216-service-account-warning
arwas11 d6e0e21
Update warning to check default as an input for cloud functions servi…
arwas11 3db974b
Merge remote-tracking branch 'origin/b398853216-service-account-warni…
arwas11 6bc190a
Merge remote-tracking branch 'origin/main' into b398853216-service-ac…
arwas11 62175ae
update warning wording and test
arwas11 ec3e09e
Merge remote-tracking branch 'origin/main' into b398853216-service-ac…
arwas11 71a7069
Merge branch 'main' into b398853216-service-account-warning
arwas11 d820fc6
Merge branch 'main' into b398853216-service-account-warning
arwas11 2244ab0
Apply suggestions from code review
tswast 46f295b
speed up warning test
tswast 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 hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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 sys | ||
import tempfile | ||
import textwrap | ||
import warnings | ||
|
||
import google.api_core.exceptions | ||
from google.cloud import bigquery, functions_v2, storage | ||
|
@@ -1359,6 +1360,51 @@ def square_num(x): | |
) | ||
|
||
|
||
@pytest.mark.flaky(retries=2, delay=120) | ||
def test_remote_function_warns_default_cloud_function_service_account(scalars_dfs): | ||
project = "bigframes-dev-perf" | ||
|
||
gcf_service_account = "default" | ||
|
||
rf_session = bigframes.Session(context=bigframes.BigQueryOptions(project=project)) | ||
|
||
try: | ||
with warnings.catch_warnings(record=True) as w: | ||
tswast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
warnings.simplefilter("always") | ||
|
||
@rf_session.remote_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. I think we don't actually have to deploy a function to test this warning. Will try something and add a commit if so. |
||
[int], | ||
int, | ||
reuse=False, | ||
cloud_function_service_account=gcf_service_account, | ||
tswast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
def square_num(x): | ||
if x is None: | ||
return x | ||
return x * x | ||
|
||
scalars_df, scalars_pandas_df = scalars_dfs | ||
|
||
bf_int64_col = scalars_df["int64_col"] | ||
bf_result_col = bf_int64_col.apply(square_num) | ||
bf_result = bf_int64_col.to_frame().assign(result=bf_result_col).to_pandas() | ||
|
||
pd_int64_col = scalars_pandas_df["int64_col"] | ||
pd_result_col = pd_int64_col.apply(lambda x: x if x is None else x * x) | ||
pd_result = pd_int64_col.to_frame().assign(result=pd_result_col) | ||
|
||
assert_pandas_df_equal(bf_result, pd_result, check_dtype=False) | ||
|
||
if len(w) > 0: | ||
assert issubclass(w[0].category, FutureWarning) | ||
assert "You have not explicitly set a user-managed" in str(w[0].message) | ||
tswast marked this conversation as resolved.
Show resolved
Hide resolved
|
||
finally: | ||
# clean up the gcp assets created for the remote function | ||
cleanup_remote_function_assets( | ||
rf_session.bqclient, rf_session.cloudfunctionsclient, square_num | ||
) | ||
|
||
|
||
@pytest.mark.flaky(retries=2, delay=120) | ||
def test_remote_function_with_gcf_cmek(): | ||
# TODO(shobs): Automate the following set-up during testing in the test project. | ||
|
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.
Uh oh!
There was an error while loading. Please reload this page.