deep copy domain.Config object before returning it#1701
deep copy domain.Config object before returning it#1701cppforlife wants to merge 1 commit intoknative:masterfrom
Conversation
google-prow-robot
added
the
size/M
|
@mattmoor any idea if pull-knative-serving-integration-tests flaked or legit failure? |
|
The integration tests timed out. They're taking more than 10 minutes to run. |
Currently 10 minutes may not be enough, for example as in #1701.
|
/retest |
|
/lgtm |
|
Nice work @cppforlife! I wonder where else this antipattern occurs? ;-) |
|
This fix is not necessary. See #1680 (comment). /close |
|
After discussion with @cppforlife and @Zteve, it seems best to be defensive. Re-opening. |
|
The following is the coverage report on pkg/.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cppforlife, mattmoor 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 |
google-prow-robot
added
the
approved
|
@cppforlife: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Currently 10 minutes may not be enough, for example as in knative#1701.
|
@cppforlife any chance of fixing the conflicts so we can get this in? |
…2208) * Fix SSH keys workaround for kubetest Create the ~/.ssh dir if it doesn't exist, don't assume it always exist. Backported from knative/test-infra#151 * Fix authentication for test clusters Instead of relying on default options, use basic authentication for test cluster. Also make acquire_cluster_admin_role() handle auth through certificates, since it's used also on deployment. Backport of knative/test-infra#115 * Increase E2E tests timeout to 20 minutes Currently 10 minutes may not be enough, for example as in #1701. Backport of #1702. * Drop Dockerfile in test images * removed dockerfiles * moved test images to the same location * adjust repo * fix command and docs * update upload-test-images.sh * preserve import paths * fix appYaml path * preserve path in upload-test-images.sh Backport of #1792.
|
I believe this is replaced by the work @dprotaso did with the config store. |
|
Also #1680 wasn't valid after all. |
7 participants
One more fix for #1680