Henry Chen

Results 13 comments of Henry Chen

I suspect this change was made for performance optimizations or to simplify the state transitions. That said, I agree the current behavior feels unintuitive.

> thanks for the PR, please update the before/after (it seems to be outdated) updated!

> > > thanks for the PR, please update the before/after (it seems to be outdated) > > > > > > updated! > > It is still outdated: >...

> @henry3260 this change broke few test: > > ``` > === RUN TestDownloadOnly/v1.28.0/binaries > aaa_download_only_test.go:157: expected the file for binary exist at "/home/jenkins/minikube-integration/21664-14293/.minikube/cache/linux/amd64/v1.28.0/kubelet" but got error stat /home/jenkins/minikube-integration/21664-14293/.minikube/cache/linux/amd64/v1.28.0/kubelet: no...

Do you think we should introduce a helper function like `CachedBinaryPath` inside `t.Run("cached-images", ...)` to keep things consistent? If so, should we open a separate issue for that improvement, or...

> @henry3260 looking in the test is does not disable preloads, and it checks if reload exists after starting minikube. If the preload tarball exists it does check for binaries....

> Nice! > > It would be nice to add `pytest.mark.parametrize` with slash value key in `task-sdk-integration-tests/tests/task_sdk_tests/test_xcom_operations.py` tests. > > Thanks! Thanks for reviewing! Slash keys aren’t supported today: the...

> @henry3260 Do you mind rebasing to fix the conflicts. We can probably go ahead and merge, the PR is looking good. Sure

> @henry3260 Do you mind rebasing to fix the conflicts. We can probably go ahead and merge, the PR is looking good. All CI tests have passed. Ready for merge!