refactor: fix invalid escape sequences #11147
Conversation
google-oss-prow
bot
requested review from
animeshsingh,
DharmitD,
gaoning777,
HumairAK,
ojarjur,
ryansteakley and
zijianjoy
|
Hi @TheKevJames. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions 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. |
|
/ok-to-test |
|
@hbelmiro any way for me to see the logs of why that test failed? Looks like might be infra issues, but want to double-check it's not a bug in my changes. |
@TheKevJames we only have this. It's failing on several PRs and it seems to be an infra issue indeed. If you want to try it locally, you can try the following. I have it on my notes, but I'm not sure it still works. source_root=$(pwd)
pip install --upgrade pip
pip install $source_root/sdk/python
pushd api
make clean python
popd
python3 -m pip install api/v2alpha1/python
pip install components/google-cloud
pip install $(grep 'pytest==' sdk/python/requirements-dev.txt)
pytest test/gcpc-tests/run_all_gcpc_modules.py |
|
Needed to update your instructions to account for some changes (you need to install protoc, wheel, and fix protobuf's broken packaging), pasted my terminal log below in case you want to update your notes. Test succeeded locally. |
|
Thank you for sharing this @TheKevJames. |
|
Looks like the updated test is in, rebased. |
|
@hbelmiro would you please be able to mark this for testing? Thank you! |
|
/ok-to-test |
|
I see the tests have been failing for all commits on mainline as well. I don't think any of these are issues with my code? @hbelmiro how do y'all prefer to tackle this? I see other PRs are still getting merged despite the failures, not sure what the approach is for that. Otherwise, I guess we can wait until mainline is green and I can rebase again? Please advise. |
@TheKevJames
This should not be happening. There's no point of having tests if we ignore them. |
|
Sounds good, please poke me when master is green again and I'd be happy to update! Looks like "most of them" for the past few days, looking at the mainline commit graph shows every merged PR has failed / continued to fail: https://github.com/kubeflow/pipelines/commits/master/ Looking at the four from today:
|
Sure.
There's a slight difference between merging PRs with failing tests and tests failing after PRs get merged.
None of those PRs were merged with failing tests (the first case I mentioned above). We don't run all the tests on PRs. We run only the ones that may be affected by the PR. - '.github/workflows/e2e-test.yml'
- 'scripts/deploy/github/**'
- 'go.mod'
- 'go.sum'
- 'backend/**'
- 'frontend/**'
- 'proxy/**'
- 'manifests/kustomize/**'
- 'test/**' |
|
@TheKevJames the CI is green again. Can you please rebase? |
Signed-off-by: Kevin James <KevinJames@thekev.in>
|
@hbelmiro done! |
|
/ok-to-test |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chensun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Kevin James <KevinJames@thekev.in> Signed-off-by: zed546213 <zed546213@gmail.com>
Signed-off-by: Kevin James <KevinJames@thekev.in>
Signed-off-by: Kevin James <KevinJames@thekev.in>
Description of your changes:
This PR fixes SyntaxError issues in Python3.12 and deprecations in previous versions.
In python3.12, using escape sequences outside of raw strings has been moved from a deprecation warning into a syntax error. As such, I've added a linter to track down cases where we still have this issue and fixed all said reported issues.
The original list was as follows:
Checklist: