Migrating Google AutoML example_dags to sys tests #32368
Conversation
boring-cyborg
bot
added
area:providers
area:system-tests
provider:google
| ENV_ID = os.environ.get("SYSTEM_TESTS_ENV_ID") | ||
| DAG_ID = "example_automl_classification" | ||
| GCP_PROJECT_ID = os.environ.get("SYSTEM_TESTS_GCP_PROJECT", "default") | ||
| GCP_AUTOML_LOCATION = "us-central1" |
There was a problem hiding this comment.
Should we hardcode these values? Shouldn't it be fetched from env var?
There was a problem hiding this comment.
I was following the convention of the other system tests written, for example in #30003 and also in the automl folder. Does it make sense to auto hard code them?
There was a problem hiding this comment.
cc: @VladaZakharova might help here.
There was a problem hiding this comment.
The main principle is to keep these tests self-sufficient. But they need an external resource and infra on which those resources run. We can control those resources, like creating a bucket on the fly, but if a user has a certain infra, IMO it would be best to give flexibility to the user to set up the test given the infra rather than looking at the test and setting up the infra. That would end up making these tests less useful wdyt?
There was a problem hiding this comment.
Nice point. Any suggestions that i can follow? It will help me tweak the DAG according to your use cases
There was a problem hiding this comment.
Generally, all the tests look good, except for the first point.
There was a problem hiding this comment.
Thanks. Do you mean to say that we should have it as:
GCP_PROJECT_ID = os.environ.get("GCP_PROJECT_ID", "your-project-id")
GCP_AUTOML_LOCATION = os.environ.get("GCP_AUTOML_LOCATION", "us-central1")
GCP_AUTOML_TEXT_CLS_BUCKET = os.environ.get("GCP_AUTOML_TEXT_CLS_BUCKET", "gs://INVALID BUCKET NAME")
And so on?
|
@Adaverse I have handled your review comments in the newest commit. |
| # create_dataset_task >> delete_datasets_task | ||
|
|
There was a problem hiding this comment.
Seems like we need a watcher wdyt?
There was a problem hiding this comment.
Sure, we can have a watcher too for each of these tests
--------- Co-authored-by: Amogh Desai <adesai@adesai-MBP16.local>
related: #32295
Moving the
example_dagsfor google cloud sensors to sys tests.This is a step by step effort to migrate all the google cloud tests to sys tests.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.