-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(api): add new params BaseBranch
, Commit
#3985
base: main
Are you sure you want to change the base?
Conversation
@jamengual 2nd try – this makes HeadCommit and BaseBranch optional parameters. If they are missing, we fallback to the existing behavior which I guess works fine as long as your plan workflow consist of one step only. I wonder if the real issue is the fact that there's an attempt to re-clone on every step within the same workflow, if that wasn't happening things would work (I think). |
lgtm @rgs1 can you also add some tests as well? |
Thanks for the review @chenrui333. Just pushed an updated commit with the existing test updated, so that it exercises the new fields. |
108743b
to
24c631c
Compare
As of now, the /api/plan endpoint constructs a PullRequest that uses the provided Ref parameter as the base branch and head commit. This causes issues when running additional steps along with plan (e.g.: policy check, infracost, etc). Here's an example error: ``` repo was already cloned but is not at correct commit, wanted \"pr-plan-test\" got \"7a19f2011...\" ``` This will then trigger a new clone which will override needed artifacts from the previous steps which will then cause /api/plan to fail. By properly setting HeadCommit from the new optional APIRequest parameter this issue is avoided. Signed-off-by: Raul Gutierrez Segales <[email protected]>
/retest |
BaseBranch
, Commit
@rgs1 could you fix the conflicts please? |
Signed-off-by: Simon Heather <[email protected]>
@rgs1, the tests are failing. Are you able to take a look? |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
@rgs1 can you check the tests? Thanks. |
what
BaseBranch
andCommit
why
As of now, the /api/plan endpoint constructs a PullRequest that uses the provided Ref parameter as the base branch and head commit. This causes issues when running additional steps along with plan (e.g.: policy check, infracost, etc).
Here's an example error:
This will then trigger a new clone which will override needed artifacts from the previous steps which will then cause /api/plan to fail.
By properly setting HeadCommit from the new optional APIRequest parameter this issue is avoided.
references
tests