Skip to content
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

Use repository settings from piped config #4739

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

golemiso
Copy link
Contributor

@golemiso golemiso commented Dec 26, 2023

What this PR does / why we need it:

  • planner/scheduler should follow repository settings in piped config.
  • plan preview builder should not check Remote and Branch because repository settings in app config metadata are not reliable (they can be outdated by modifying piped config)

Which issue(s) this PR fixes:

Fixes #4618

Does this PR introduce a user-facing change?:

  • How are users affected by this change: None
  • Is this breaking change: No
  • How to migrate (if breaking change): None

Copy link

codecov bot commented Dec 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.82%. Comparing base (1864cdb) to head (530752a).
Report is 96 commits behind head on master.

❗ Current head 530752a differs from pull request most recent head dab442a. Consider uploading reports for the commit dab442a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4739      +/-   ##
==========================================
- Coverage   30.84%   30.82%   -0.02%     
==========================================
  Files         222      221       -1     
  Lines       26037    26011      -26     
==========================================
- Hits         8032     8019      -13     
+ Misses      17355    17342      -13     
  Partials      650      650              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ffjlabo
Copy link
Member

ffjlabo commented Dec 28, 2023

Thank you for the PR :)

Actually, this change may affect the behavior of executing the pipeline.
We need to check and discuss the effect of it.
So it may take more time to examine the effect. 🙏

@ffjlabo
Copy link
Member

ffjlabo commented Jan 5, 2024

📝 We should investigate the effect because some features including Deployment are based on the information of Application.GtiPath.
https://github.com/pipe-cd/pipecd/blob/master/pkg/model/application.pb.go#L110C6-L110C6

Copy link

github-actions bot commented Feb 5, 2024

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added Stale and removed Stale labels Feb 5, 2024
Signed-off-by: golemiso <3282656+golemiso@users.noreply.github.com>
Signed-off-by: golemiso <3282656+golemiso@users.noreply.github.com>
@golemiso
Copy link
Contributor Author

golemiso commented Mar 7, 2024

refactored: override repository with piped config (decent one) when creating deployment.
I suppose this can reduce side effects.

Copy link

github-actions bot commented Apr 7, 2024

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Copy link

github-actions bot commented May 8, 2024

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployment fails when the application git branch is changed
3 participants