chore: fix marking of async upload job e2e and integration tests#2604
chore: fix marking of async upload job e2e and integration tests#2604google-oss-prow[bot] merged 3 commits intokubeflow:mainfrom
Conversation
Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
jonburdo
left a comment
jonburdo
left a comment
There was a problem hiding this comment.
/lgtm
/approve
/hold
A couple comments that I think are worth address. If we're doing more work on these right away, they could be fixed in a follow-up, so feel free to merge as-is.
Feel free to unhold if you are ready to merge
| return os.getenv("AUTH_TOKEN", None) # type: ignore[arg-type] | ||
|
|
||
| @pytest.fixture(scope="session") | ||
| def verify_ssl() -> bool: |
There was a problem hiding this comment.
This fixture is unused. If you want to avoid adding args to every function that needs this, you could just make this a function and call it where needed instead of inline:
verify_env = os.environ.get("VERIFY_SSL")
verify = verify_env.lower() == "true" if verify_env is not None else NoneAlso it's return type is bool but it can return None
There was a problem hiding this comment.
Since using it in only one place under jobs/async-upload, used it directly there without creating a utility. If this area adds more tests, then would definitely consider creating a utility function.
…oad.py Co-authored-by: Jon Burdo <jon@jonburdo.com> Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
| job_name: str, namespace: str, k8s_batch_client: kubernetes.client.BatchV1Api, timeout_seconds: int = 60 | ||
| job_name: str, namespace: str, k8s_batch_client: kubernetes.client.BatchV1Api, timeout_seconds: int = 120 |
There was a problem hiding this comment.
Good call! Should help with intermittent failures due to timeout
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jonburdo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
…eflow#2604) * chore: fix marking of async upload job e2e and integration tests Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> * Update jobs/async-upload/tests/integration/test_integration_async_upload.py Co-authored-by: Jon Burdo <jon@jonburdo.com> Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> * fix: address review comments Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> --------- Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> Co-authored-by: Jon Burdo <jon@jonburdo.com>
2 participants
Description
How Has This Been Tested?
Merge criteria:
DCOcheck)ok-to-testhas been added to the PR.If you have UI changes