-
-
Notifications
You must be signed in to change notification settings - Fork 375
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
Fix skipped pipelines model #2923
Conversation
Don't know if this is a good way to fix it. Please let me know what you think. |
If I am correct those pipelines were skipped / not saved at all before, so we might consider to even remove them again? |
Correct all pipelines are saved to the DB at https://github.com/woodpecker-ci/woodpecker/pull/2923/files#diff-80265a1351d5d4936ff03f75e08517b6cfcca8015c02e730ebd90408983a0200R58 as required to populate all env vars to the step builder In the case pipelines are empty, we can also remove them from the DB again. Writing to the DB and remove it again afterward feels wrong in general, as it creates unnecessary DB calls. |
Agree. Alternative would be to parse the pipeline, store it and set the pipeline env vars like id afterwards. |
Yep would need a bit more work but I think thats the better way. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2923 +/- ##
==========================================
- Coverage 33.89% 33.89% -0.01%
==========================================
Files 219 219
Lines 13829 13847 +18
==========================================
+ Hits 4688 4693 +5
- Misses 8783 8796 +13
Partials 358 358 ☔ View full report in Codecov by Sentry. |
I would look into the alternative approach first. |
After looking into it again I have not found a solution yet. The env var handling for pipeline workflows is tied together with the substitution logic and the workflow metadata and I was not able to decouple it to (re)call it after creating the pipeline. I would need help here. |
Yes, there's a pretty big issue here. We can try to just delete the pipeline if it's skipped but that wouldn't be really clean. |
Ok I would say we just delete empty pipelines from the DB for now as a hotfix and I create a followup issue. What do you think? |
The PR deletes the pipeline from the DB again after checking if it was skipped. To avoid leftovers in the DB (e.g. by interrupted processes, errors during the deletion etc.) I have added a new state Personally I think I prefer the alternative approach from #2928 but happy to discuss. |
As discussed in #2928, as a hotfix this approach is preferred. |
Co-authored-by: Lauris BH <lauris@nix.lv>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As quickfix this is ok with me before we find a better way to fix this
Fixes a bug introduced in #2923. I'll also try to add a test case.
## [2.1.0](https://github.com/woodpecker-ci/woodpecker/releases/tag/2.1.0) - 2023-12-26 ### ✨ Features - Add pull request closed event [[#2684](#2684)] - Add depends_on support for steps [[#2771](#2771)] - gitlab: support nested repos [[#2981](#2981)] - Support go plugins for forges and agent backends [[#2751](#2751)] ### 📈 Enhancement - Show default branch on top [[#3019](#3019)] - Support more addon types [[#2984](#2984)] - Hide PR tab if PRs are disabled [[#3004](#3004)] - Switch to ULID [[#2986](#2986)] - Ignore pipelines without config [[#2949](#2949)] - Link labels to input and select [[#2974](#2974)] - Register Agent with hostname [[#2936](#2936)] - Update slogan & logo [[#2962](#2962)] - Improve error handling when activating a repository [[#2965](#2965)] - Add check for storage where repo/org name is empty [[#2968](#2968)] - Update pipeline icons [[#2783](#2783)] - Kubernetes refactor [[#2794](#2794)] - Export changed files via builtin environment variables [[#2935](#2935)] - Show secrets from org and global level [[#2873](#2873)] - Only update pipelineStatus in one place [[#2952](#2952)] - Rename `engine` to `backend` [[#2950](#2950)] - Add linting for `log.Fatal()` [[#2946](#2946)] - Remove separate root path config [[#2943](#2943)] - init CI_COMMIT_TAG if commit ref is a tag [[#2934](#2934)] - Update go module path for major version 2 [[#2905](#2905)] - Unify date/time dependencies [[#2891](#2891)] - Add linting for `any` [[#2893](#2893)] - Fix vite deprecations [[#2885](#2885)] - Migrate to Xormigrate [[#2711](#2711)] - Simple security context options (Kubernetes) [[#2550](#2550)] - Changes PullRequest Index to ForgeRemoteID type [[#2823](#2823)] ### 🐛 Bug Fixes - Hide queue visualization if nothing to show [[#3003](#3003)] - fix and lint swagger file [[#3007](#3007)] - Fix IPv6 host aliases for kubernetes [[#2992](#2992)] - Fix cli lint throwing error on warnings [[#2995](#2995)] - Fix static file caching [[#2975](#2975)] - Gitea driver: ignore GetOrg error if we get a valid user. [[#2967](#2967)] - feat(k8s): Add a port name to service definition [[#2933](#2933)] - Fix error container overflow [[#2957](#2957)] - ignore some errors on repairAllRepos [[#2792](#2792)] - Allow to restart pipelines that has warnings [[#2939](#2939)] - Fix skipped pipelines model [[#2923](#2923)] - fix: Add `backend_options` to service linter entry [[#2930](#2930)] - Fix flags added multiple times [[#2914](#2914)] - Fix schema validation with array syntax for clone and services [[#2920](#2920)] - Fix prometheus docs [[#2919](#2919)] - Fix podman agent container in v2 [[#2897](#2897)] - Fix bitbucket org fetching [[#2874](#2874)] - Only deploy docs on `main` [[#2892](#2892)] - Fix pipeline-related environment [[#2876](#2876)] - Fix version check partially [[#2871](#2871)] - Fix unregistering agents when using agent tokens [[#2870](#2870)] ### 📚 Documentation - [Awesome Woodpecker] added yet another autoscaler [[#3011](#3011)] - Add cookbook blog and improve docs [[#3002](#3002)] - Replace multi-pipelines with workflows on docs frontpage [[#2990](#2990)] - Update README badges [[#2956](#2956)] - Update 20-kubernetes.md [[#2927](#2927)] - Add release documentation to CONTRIBUTING [[#2917](#2917)] - Add nix-attic plugin to the index [[#2889](#2889)] - Add usage with Tunnelmole to docs [[#2881](#2881)] - Improve code blocks in docs [[#2879](#2879)] - Add a blog post [[#2877](#2877)] - Add documentation on Kubernetes securityContext [[#2822](#2822)] - Add default page to categories [[#2869](#2869)] - Use same format for Github docs as used for the other forges [[#2866](#2866)] ### Misc - chore(deps): update dependency isomorphic-dompurify to v2 [[#3001](#3001)] - fix(deps): update dependency @intlify/unplugin-vue-i18n to v2 [[#2998](#2998)] - Fix go in gitpod [[#2973](#2973)] - fix(deps): update module google.golang.org/grpc to v1.60.1 [[#2969](#2969)] - chore(deps): update docker.io/alpine docker tag to v3.19 [[#2970](#2970)] - Fix broken gated repos [[#2959](#2959)] - fix(deps): update golang (packages) [[#2958](#2958)] - Update docker.io/techknowlogick/xgo Docker tag to go-1.21.5 [[#2926](#2926)] - Update docker.io/golang Docker tag to v1.21.5 [[#2925](#2925)] - Lock file maintenance [[#2910](#2910)] - Update web npm deps non-major [[#2909](#2909)] - Update docs npm deps non-major [[#2908](#2908)] - Update golang (packages) [[#2904](#2904)] - Update module github.com/google/go-github/v56 to v57 [[#2899](#2899)] - Update dependency marked to v11 [[#2898](#2898)] - Update dependency vite-svg-loader to v5 [[#2837](#2837)] - Update golang (packages) [[#2894](#2894)] - Update web npm deps non-major [[#2895](#2895)] - Update web npm deps non-major [[#2884](#2884)] - Update docker.io/woodpeckerci/plugin-docker-buildx Docker tag to v2.2.1 [[#2883](#2883)]
Fixes: #2901