fix click_deploy_test #2400
fix click_deploy_test #2400
Conversation
| type=str, | ||
| help="e2e test project id") | ||
| parser.add_argument( | ||
| "--project_number", | ||
| default="453914067825", | ||
| default="29647740582", |
There was a problem hiding this comment.
Why do we need project_number? Is this the same project_number as kubeflow-ci-deployment?
There was a problem hiding this comment.
It's the same. project id and number are both passed as request. @kunmingg is it needed?
There was a problem hiding this comment.
Yes, currently we pass both project id and number to deploy API, which can be simplified.
| @@ -25,7 +25,7 @@ local artifactsDir = outputDir + "/artifacts"; | |||
| // Source directory where all repos should be checked out | |||
| local srcRootDir = testDir + "/src"; | |||
| // The directory containing the kubeflow/kubeflow repo | |||
| local srcDir = srcRootDir + "/kubeflow/kubeflow"; | |||
| local srcDir = srcRootDir + "/github.com/kubeflow/kubeflow"; | |||
There was a problem hiding this comment.
Is this diverging from every other test? I think for every other workflow we use
/src/{OWNER}/{REPO}
Is this because we need to set the go path?
There was a problem hiding this comment.
I would suggest adding github.com to srcRootDir and then its more consistent with other workflows.
| type=str, | ||
| help="e2e test project id") | ||
| parser.add_argument( | ||
| "--project_number", | ||
| default="453914067825", | ||
| default="29647740582", |
There was a problem hiding this comment.
Yes, currently we pass both project id and number to deploy API, which can be simplified.
|
@kunmingg yes , I was running it in new project |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kunmingg 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 |
* testing build image * fix * fix * fix * fix * address comment
For #2364
For kubeflow/testing#301
This change is