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 forge from db #1417

Merged
merged 59 commits into from
Apr 16, 2024
Merged

Use forge from db #1417

merged 59 commits into from
Apr 16, 2024

Conversation

anbraten
Copy link
Member

@anbraten anbraten commented Nov 15, 2022

This is the first step towards support for multiple forges (#138). It inserts a forge using the currently existing env varaibles into db and uses this forge from db later on in all places of the code.

closes #621

addresses #138

TODO

  • add forges table
  • add id of forge to repo
  • use forge of repo
  • add forge from env vars to db if not exists
  • migrate repo.ForgeID to the newly generated forge
  • support cache with forge from repo
  • maybe add forge loading cache? (use LRU cache for forges, I expect users to have less than 10 forges normally)

@anbraten anbraten changed the title use forge from db Use forge from db Nov 15, 2022
@anbraten anbraten added this to the 1.0.0 milestone Nov 15, 2022
@anbraten anbraten added the enhancement improve existing features label Nov 15, 2022
@anbraten anbraten mentioned this pull request Nov 15, 2022
6 tasks
@6543 6543 modified the milestones: 1.0.0, 1.1.0 Jun 10, 2023
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Oct 9, 2023

Deployment of preview was torn down

@pat-s pat-s modified the milestones: 2.0.0, 2.x.x Oct 13, 2023
@anbraten anbraten added the server label Nov 4, 2023
Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm, but didn't test

@anbraten anbraten requested a review from qwerty287 April 15, 2024 17:34
@anbraten
Copy link
Member Author

Did some more testing and tweaking. Works quite fine for me now.

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looks good, but didn't test

@anbraten
Copy link
Member Author

Still looks good, but didn't test

Do you want to test it?

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

Attention: Patch coverage is 9.70149% with 242 lines in your changes are missing coverage. Please review.

Project coverage is 35.91%. Comparing base (7816288) to head (e255378).
Report is 2 commits behind head on main.

❗ Current head e255378 differs from pull request most recent head 811bc86. Consider uploading reports for the commit 811bc86 to get more accurate results

Files Patch % Lines
server/api/repo.go 0.00% 50 Missing ⚠️
server/api/login.go 0.00% 21 Missing ⚠️
server/pipeline/create.go 0.00% 16 Missing ⚠️
server/api/cron.go 0.00% 15 Missing ⚠️
server/api/org.go 0.00% 15 Missing ⚠️
server/api/pipeline.go 0.00% 15 Missing ⚠️
server/pipeline/restart.go 0.00% 10 Missing ⚠️
server/pipeline/approve.go 0.00% 9 Missing ⚠️
...re/datastore/migration/030_set_default_forge_id.go 35.71% 6 Missing and 3 partials ⚠️
server/pipeline/decline.go 0.00% 8 Missing ⚠️
... and 15 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1417      +/-   ##
==========================================
- Coverage   35.98%   35.91%   -0.08%     
==========================================
  Files         231      233       +2     
  Lines       15599    15670      +71     
==========================================
+ Hits         5614     5628      +14     
- Misses       9569     9619      +50     
- Partials      416      423       +7     

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

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some basic testing, seems working

@anbraten anbraten enabled auto-merge (squash) April 16, 2024 05:59
@anbraten anbraten merged commit d494b6a into woodpecker-ci:main Apr 16, 2024
7 checks passed
@anbraten anbraten deleted the repo-forge branch April 16, 2024 06:05
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 16, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include forge type in repo model
6 participants