-
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
Conversation
tests/system/conftest.py
Outdated
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks.
tests/system/conftest.py
Outdated
@@ -21,6 +21,7 @@ | |||
import typing | |||
from typing import Dict, Optional | |||
|
|||
from google.api_core.exceptions import NotFound, ResourceExhausted |
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.
tests/system/utils.py
Outdated
import numpy as np | ||
import pandas as pd | ||
import pyarrow as pa # type: ignore | ||
import pytest | ||
|
||
from bigframes.functions.remote_function import get_remote_function_locations |
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.
Same here. Import packages/modules, not functions.
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!
* test: do GCF cleanup in both presubmit and e2e tests * use functions client from session * address review comments --------- Co-authored-by: Tim Sweña (Swast) <[email protected]>
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes internal issue 328515697 🦕