Fix product test filtering on master#14534
Conversation
Don't ignore product test launcher failures in product test matrix filtering script. Produce an empty product test matrix if all items are excluded and ignore it in CI.
| if: steps.filter.outputs.product-tests == 'false' && !contains(github.event.pull_request.labels.*.name, 'tests:all') && !contains(github.event.pull_request.labels.*.name, 'product-tests:all') | ||
| # all these envs are required to be set by some product test environments | ||
| env: | ||
| ABFS_CONTAINER: |
There was a problem hiding this comment.
@findinpath @alexjo2144 just FYI, it's a bit weird we have to provide so many variables just to describe the environment:
DATABRICKS_73_JDBC_URL= \
DATABRICKS_91_JDBC_URL= \
DATABRICKS_104_JDBC_URL= \
DATABRICKS_LOGIN= DATABRICKS_TOKEN= \
AWS_REGION= \
S3_BUCKET= \
DATABRICKS_AWS_ACCESS_KEY_ID= \
DATABRICKS_AWS_SECRET_ACCESS_KEY= \
testing/bin/ptl suite describe --suite suite-delta-lake-databricks --config config-defaultbut I do realize using requireNonNull allows for providing an actionable error message. I'm not sure how to delay getting the actual values to when the env is started, not just created, so we can keep this as is.
There was a problem hiding this comment.
If someone adds a new environment variable and forgets to add it here, what happens? Should we fail the build in that case?
There was a problem hiding this comment.
Yes, I made sure failures like that are not silently ignored.
There was a problem hiding this comment.
DATABRICKS_XX_JDBC_URL corresponds to the Databricks cluster. There are 3 clusters we check for compatibility against. The suite contains correspondingly 3 environments against which we test Delta Lake.
AWS related settings are needed for testing against AWS S3 & Glue (the metastore for the Delta Lake tests).
If someone adds a new environment variable and forgets to add it here, what happens ?
If the env variable is not related to the product tests it will simply be ignored.
|
This is a follow up to #10984 |
|
Thanks for the quick fix. |
3 participants
Description
The product tests job currently fails on master because when building the matrix we try to describe all environments and some require specific environmental variables to be set. This PR sets them to empty strings but also makes the script more robust by:
Non-technical explanation
n/a
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: