-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Migrate Python tooling to uv and ruff #1265
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,7 +31,8 @@ | |||||||||||
| # [START savegoogletoken] | ||||||||||||
| @identity_fn.before_user_created() | ||||||||||||
| def savegoogletoken( | ||||||||||||
| event: identity_fn.AuthBlockingEvent) -> identity_fn.BeforeCreateResponse | None: | ||||||||||||
| event: identity_fn.AuthBlockingEvent, | ||||||||||||
| ) -> identity_fn.BeforeCreateResponse | None: | ||||||||||||
| """During sign-up, save the Google OAuth2 access token and queue up a task | ||||||||||||
| to schedule an onboarding session on the user's Google Calendar. | ||||||||||||
|
|
||||||||||||
|
|
@@ -48,24 +49,19 @@ def savegoogletoken( | |||||||||||
| doc_ref.set({"calendar_access_token": event.credential.access_token}, merge=True) | ||||||||||||
|
|
||||||||||||
| tasks_client = google.cloud.tasks_v2.CloudTasksClient() | ||||||||||||
| task_queue = tasks_client.queue_path(params.PROJECT_ID.value, | ||||||||||||
| options.SupportedRegion.US_CENTRAL1, | ||||||||||||
| "scheduleonboarding") | ||||||||||||
| task_queue = tasks_client.queue_path( | ||||||||||||
| params.PROJECT_ID.value, options.SupportedRegion.US_CENTRAL1, "scheduleonboarding" | ||||||||||||
| ) | ||||||||||||
| target_uri = get_function_url("scheduleonboarding") | ||||||||||||
| calendar_task = google.cloud.tasks_v2.Task(http_request={ | ||||||||||||
| "http_method": google.cloud.tasks_v2.HttpMethod.POST, | ||||||||||||
| "url": target_uri, | ||||||||||||
| "headers": { | ||||||||||||
| "Content-type": "application/json" | ||||||||||||
| calendar_task = google.cloud.tasks_v2.Task( | ||||||||||||
| http_request={ | ||||||||||||
| "http_method": google.cloud.tasks_v2.HttpMethod.POST, | ||||||||||||
| "url": target_uri, | ||||||||||||
| "headers": {"Content-type": "application/json"}, | ||||||||||||
| "body": json.dumps({"data": {"uid": event.data.uid}}).encode(), | ||||||||||||
| }, | ||||||||||||
| "body": json.dumps({ | ||||||||||||
| "data": { | ||||||||||||
| "uid": event.data.uid | ||||||||||||
| } | ||||||||||||
| }).encode() | ||||||||||||
| }, | ||||||||||||
| schedule_time=datetime.now() + | ||||||||||||
| timedelta(minutes=1)) | ||||||||||||
| schedule_time=datetime.now() + timedelta(minutes=1), | ||||||||||||
| ) | ||||||||||||
| tasks_client.create_task(parent=task_queue, task=calendar_task) | ||||||||||||
| # [END savegoogletoken] | ||||||||||||
|
|
||||||||||||
|
|
@@ -79,46 +75,48 @@ def scheduleonboarding(request: tasks_fn.CallableRequest) -> https_fn.Response: | |||||||||||
| """ | ||||||||||||
|
|
||||||||||||
| if "uid" not in request.data: | ||||||||||||
| return https_fn.Response(status=https_fn.FunctionsErrorCode.INVALID_ARGUMENT, | ||||||||||||
| response="No user specified.") | ||||||||||||
| return https_fn.Response( | ||||||||||||
| status=https_fn.FunctionsErrorCode.INVALID_ARGUMENT, response="No user specified." | ||||||||||||
| ) | ||||||||||||
| uid = request.data["uid"] | ||||||||||||
|
|
||||||||||||
| user_record: auth.UserRecord = auth.get_user(uid) | ||||||||||||
| if user_record.email is None: | ||||||||||||
| return https_fn.Response(status=https_fn.FunctionsErrorCode.INVALID_ARGUMENT, | ||||||||||||
| response="No email address on record.") | ||||||||||||
| return https_fn.Response( | ||||||||||||
| status=https_fn.FunctionsErrorCode.INVALID_ARGUMENT, | ||||||||||||
| response="No email address on record.", | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| firestore_client: google.cloud.firestore.Client = firestore.client() | ||||||||||||
| user_info = firestore_client.collection("user_info").document(uid).get().to_dict() | ||||||||||||
| if not isinstance(user_info, dict) or "calendar_access_token" not in user_info: | ||||||||||||
| return https_fn.Response(status=https_fn.FunctionsErrorCode.PERMISSION_DENIED, | ||||||||||||
| response="No Google OAuth token found.") | ||||||||||||
| return https_fn.Response( | ||||||||||||
| status=https_fn.FunctionsErrorCode.PERMISSION_DENIED, | ||||||||||||
| response="No Google OAuth token found.", | ||||||||||||
| ) | ||||||||||||
| calendar_access_token = user_info["calendar_access_token"] | ||||||||||||
| firestore_client.collection("user_info").document(uid).update( | ||||||||||||
| {"calendar_access_token": google.cloud.firestore.DELETE_FIELD}) | ||||||||||||
| {"calendar_access_token": google.cloud.firestore.DELETE_FIELD} | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| google_credentials = google.oauth2.credentials.Credentials(token=calendar_access_token) | ||||||||||||
|
|
||||||||||||
| calendar_client = googleapiclient.discovery.build("calendar", | ||||||||||||
| "v3", | ||||||||||||
| credentials=google_credentials) | ||||||||||||
| calendar_client = googleapiclient.discovery.build( | ||||||||||||
| "calendar", "v3", credentials=google_credentials | ||||||||||||
| ) | ||||||||||||
| calendar_event = { | ||||||||||||
| "summary": "Onboarding with ExampleCo", | ||||||||||||
| "location": "Video call", | ||||||||||||
| "description": "Walk through onboarding tasks with an ExampleCo engineer.", | ||||||||||||
| "start": { | ||||||||||||
| "dateTime": (datetime.now() + timedelta(days=3)).isoformat(), | ||||||||||||
| "timeZone": "America/Los_Angeles" | ||||||||||||
| "timeZone": "America/Los_Angeles", | ||||||||||||
|
Comment on lines
112
to
+113
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. In Cloud Functions,
Suggested change
|
||||||||||||
| }, | ||||||||||||
| "end": { | ||||||||||||
| "dateTime": (datetime.now() + timedelta(days=3, hours=1)).isoformat(), | ||||||||||||
| "timeZone": "America/Los_Angeles" | ||||||||||||
| "timeZone": "America/Los_Angeles", | ||||||||||||
|
Comment on lines
116
to
+117
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. Similar to the start time, the end time should also use
Suggested change
|
||||||||||||
| }, | ||||||||||||
| "attendees": [{ | ||||||||||||
| "email": user_record.email | ||||||||||||
| }, { | ||||||||||||
| "email": "onboarding@example.com" | ||||||||||||
| }] | ||||||||||||
| "attendees": [{"email": user_record.email}, {"email": "onboarding@example.com"}], | ||||||||||||
| } | ||||||||||||
| calendar_client.events().insert(calendarId="primary", body=calendar_event).execute() | ||||||||||||
|
|
||||||||||||
|
|
@@ -137,10 +135,13 @@ def get_function_url(name: str, location: str = options.SupportedRegion.US_CENTR | |||||||||||
| The URL of the function | ||||||||||||
| """ | ||||||||||||
| credentials, project_id = google.auth.default( | ||||||||||||
| scopes=["https://www.googleapis.com/auth/cloud-platform"]) | ||||||||||||
| scopes=["https://www.googleapis.com/auth/cloud-platform"] | ||||||||||||
| ) | ||||||||||||
| authed_session = google.auth.transport.requests.AuthorizedSession(credentials) | ||||||||||||
| url = ("https://cloudfunctions.googleapis.com/v2beta/" + | ||||||||||||
| f"projects/{project_id}/locations/{location}/functions/{name}") | ||||||||||||
| url = ( | ||||||||||||
| "https://cloudfunctions.googleapis.com/v2beta/" | ||||||||||||
| + f"projects/{project_id}/locations/{location}/functions/{name}" | ||||||||||||
| ) | ||||||||||||
| response = authed_session.get(url) | ||||||||||||
| data = response.json() | ||||||||||||
| function_url = data["serviceConfig"]["uri"] | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,8 +17,7 @@ | |||||||||||||||||||
| import difflib | ||||||||||||||||||||
| import pathlib | ||||||||||||||||||||
| import re | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from yapf.yapflib import yapf_api | ||||||||||||||||||||
| import subprocess | ||||||||||||||||||||
|
|
||||||||||||||||||||
| start_tag_re = re.compile(r"^([ \t]*)#\s*\[START\s+(\w+).*\]\s*\n", flags=re.MULTILINE) | ||||||||||||||||||||
| end_tag_re = re.compile(r"^\s*#\s*\[END\s+(\w+).*\][ \t]*$", flags=re.MULTILINE) | ||||||||||||||||||||
|
|
@@ -41,19 +40,25 @@ def check_and_diff(files: list[str]) -> int: | |||||||||||||||||||
| orig = f.read() | ||||||||||||||||||||
| fmt = format(orig) | ||||||||||||||||||||
| diff = list( | ||||||||||||||||||||
| difflib.unified_diff(orig.splitlines(), | ||||||||||||||||||||
| fmt.splitlines(), | ||||||||||||||||||||
| fromfile=file, | ||||||||||||||||||||
| tofile=f"{file} (reformatted)", | ||||||||||||||||||||
| lineterm="")) | ||||||||||||||||||||
| difflib.unified_diff( | ||||||||||||||||||||
| orig.splitlines(), | ||||||||||||||||||||
| fmt.splitlines(), | ||||||||||||||||||||
| fromfile=file, | ||||||||||||||||||||
| tofile=f"{file} (reformatted)", | ||||||||||||||||||||
| lineterm="", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| if len(diff) > 0: | ||||||||||||||||||||
| diff_count += 1 | ||||||||||||||||||||
| print("\n".join(diff), end="\n\n") | ||||||||||||||||||||
| return diff_count | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def format(src: str) -> str: | ||||||||||||||||||||
| out, _ = yapf_api.FormatCode(src, style_config=pyproject_toml) | ||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||
| ["ruff", "format", "-"], input=src.encode("utf-8"), capture_output=True, check=True | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
Comment on lines
+58
to
+60
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. The
Suggested change
|
||||||||||||||||||||
| out = result.stdout.decode("utf-8") | ||||||||||||||||||||
| out = fix_region_tags(out) | ||||||||||||||||||||
| return out | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -83,15 +88,19 @@ def fix_end_tag(m: re.Match) -> str: | |||||||||||||||||||
| import argparse | ||||||||||||||||||||
|
|
||||||||||||||||||||
| argparser = argparse.ArgumentParser() | ||||||||||||||||||||
| argparser.add_argument("--check_only", | ||||||||||||||||||||
| "-c", | ||||||||||||||||||||
| action="store_true", | ||||||||||||||||||||
| help="check files and print diffs, but don't modify files") | ||||||||||||||||||||
| argparser.add_argument("--exclude", | ||||||||||||||||||||
| "-e", | ||||||||||||||||||||
| action="append", | ||||||||||||||||||||
| default=[], | ||||||||||||||||||||
| help="exclude file or glob (can specify multiple times)") | ||||||||||||||||||||
| argparser.add_argument( | ||||||||||||||||||||
| "--check_only", | ||||||||||||||||||||
| "-c", | ||||||||||||||||||||
| action="store_true", | ||||||||||||||||||||
| help="check files and print diffs, but don't modify files", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| argparser.add_argument( | ||||||||||||||||||||
| "--exclude", | ||||||||||||||||||||
| "-e", | ||||||||||||||||||||
| action="append", | ||||||||||||||||||||
| default=[], | ||||||||||||||||||||
| help="exclude file or glob (can specify multiple times)", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| argparser.add_argument("file_or_glob", nargs="+") | ||||||||||||||||||||
| args = argparser.parse_args() | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
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.
The
requests.postcall is missing a timeout. It's a best practice to always specify a timeout to prevent the function from hanging indefinitely if the external service is unresponsive.