Skip to content

Outdated bash operator import on Composer 2 Docs #11433 - #11440

Merged
leahecole merged 19 commits into
GoogleCloudPlatform:mainfrom
magar51:main
Apr 23, 2024
Merged

Outdated bash operator import on Composer 2 Docs #11433#11440
leahecole merged 19 commits into
GoogleCloudPlatform:mainfrom
magar51:main

Conversation

@magar51

@magar51magar51 commented Apr 12, 2024

Copy link
Copy Markdown
Contributor

Description

Fixes#11433

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

magar51 added 13 commits March 6, 2024 15:42
@magar51
Update samples for google-cloud-speech to improve usability
b6a3065
@magar51GitHub Enterprise
Update transcribe_streaming_v2.py
8b7530b
@magar51GitHub Enterprise
Handling IssueGoogleCloudPlatform#11248
e0dd6c4
Update samples for google-cloud-speech to improve usability
@magar51GitHub Enterprise
Merge pull requestGoogleCloudPlatform#1from magar51/feat/speech_usa…
6c457d7
…bility
Handling Issue GoogleCloudPlatform#11248
@magar51
Merge branch 'main' ofhttps://github.aexp.com/magar51/python-docs-sa…
f39ddd9
…mples
Handling Issue GoogleCloudPlatform#11248 - Update samples for google-cloud-speech to improve usability
@magar51
There are typos in a comment and a log messageGoogleCloudPlatform#11294
dfbad1e
@magar51GitHub Enterprise
Merge pull requestGoogleCloudPlatform#2from magar51/fix/typo
8fc1c1d
Fix/typo
@magar51
Outdated bash operator import on Composer 2 DocsGoogleCloudPlatform#…
cae63bc
…11433
@magar51GitHub Enterprise
Update grouping.py
ed2f3c7
@magar51GitHub Enterprise
Merge pull requestGoogleCloudPlatform#3from magar51/import_update
ef085bb
Outdated bash operator import on Composer 2 Docs GoogleCloudPlatform#11433
@magar51GitHub Enterprise
Update grouping.py
1a2a783
Outdated bash operator import on Composer 2 Docs GoogleCloudPlatform#11433
@magar51GitHub Enterprise
Update grouping.py
053c090
@magar51
Merge branch 'main' ofhttps://github.aexp.com/magar51/python-docs-sa…
8f0d4fe
…mples
Changes done
@magar51
magar51 requested review from a team as code ownersApril 12, 2024 09:38
@product-auto-labelproduct-auto-labelBot added samples Issues that are directly related to samples. api: composer Issues related to the Managed Service for Apache Airflow API. api: endpoints Issues related to the Cloud Endpoints API. api: speech Issues related to the Speech-to-Text API. labels Apr 12, 2024
holtskinner
holtskinner previously approved these changes Apr 12, 2024
View reviewed changes
georgetayloor
georgetayloor suggested changes Apr 16, 2024
View reviewed changes

@georgetayloorgeorgetayloor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outdated python operator import, change to:
from airflow.operators.python import PythonOperator

Comment threadcomposer/blog/gcp-tech-blog/unit-test-dags-cloud-build/dags/example_dag.py

from airflow.operators.bash_operator import BashOperator
from airflow.operators.bash import BashOperator
from airflow.operators.python_operator import PythonOperator

@georgetayloorgeorgetayloorApr 16, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from airflow.operators.python import PythonOperator

georgetayloor
georgetayloor reviewed Apr 16, 2024
View reviewed changes
Comment threadcomposer/blog/gcp-tech-blog/unit-test-dags-cloud-build/dags/example_dag.py
Comment on lines 21 to 23
from airflow.providers.google.cloud.operators.gcs import GCSCreateBucketOperator
from airflow.providers.google.cloud.operators.gcs import GCSDeleteBucketOperator
from airflow.providers.google.cloud.operators.gcs import GCSListObjectsOperator

@georgetayloorgeorgetayloorApr 16, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure on styling specifics, is more concise for:
from airflow.providers.google.cloud.operators.gcs import GCSCreateBucketOperator, GCSDeleteBucketOperator, GCSListObjectsOperator?

georgetayloor
georgetayloor approved these changes Apr 16, 2024
View reviewed changes

@georgetayloorgeorgetayloor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: one note on styling when importing multiple classes from same package

@kweinmeisterkweinmeister added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 16, 2024
@kokoro-teamkokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 16, 2024
@leahecole
leahecole dismissed holtskinner’s stale reviewApril 18, 2024 15:54

Tests and lint do not pass

leahecole
leahecole requested changes Apr 18, 2024
View reviewed changes

@leahecoleleahecole left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments - there are places where the BashOperator should not be modified. Additionally, please revert the changes in the endpoint and speech directories and submit them as separate PRs with linked issues. Additionally, I changed it to be a Draft PR while lint and tests are not passing. Feel free to change it back to "ready to review" once they are passing!

Comment threadcomposer/airflow_1_samples/clear_file_system_caches_dag.py Outdated
Comment threadcomposer/airflow_1_samples/gke_operator.py Outdated
Comment threadcomposer/airflow_1_samples/grouping.py Outdated
Comment threadcomposer/blog/gcp-tech-blog/unit-test-dags-cloud-build/dags/example_dag.py Outdated
@leahecole
leahecole marked this pull request as draft April 18, 2024 16:03
@leahecole
Apply suggestions from code review
fd876a5
reverting Airflow 1 changes where relevant
@leahecoleleahecole added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 23, 2024
@kokoro-teamkokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 23, 2024
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadendpoints/getting-started/main.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadendpoints/getting-started/main.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
leahecole
leahecole reviewed Apr 23, 2024
View reviewed changes
Comment threadspeech/snippets/transcribe_streaming_v2.py Outdated
@leahecoleleahecole added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed api: speech Issues related to the Speech-to-Text API. api: endpoints Issues related to the Cloud Endpoints API. labels Apr 23, 2024
@kokoro-teamkokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 23, 2024
@kweinmeisterkweinmeister added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 23, 2024
@leahecole
leahecole marked this pull request as ready for review April 23, 2024 16:25
@leahecole
leahecole merged commit 768f82b into GoogleCloudPlatform:mainApr 23, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leahecoleleahecoleleahecole approved these changes
@holtskinnerholtskinnerholtskinner left review comments
+1 more reviewer
@georgetayloorgeorgetayloorgeorgetayloor approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@leahecoleleahecole

Labels

api: composerIssues related to the Managed Service for Apache Airflow API.kokoro:runAdd this label to force Kokoro to re-run the tests.samplesIssues that are directly related to samples.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Outdated bash operator import on Composer 2 Docs

6 participants

@magar51@leahecole@holtskinner@georgetayloor@kweinmeister@kokoro-team