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

Limit repository size #7833

Closed
wants to merge 4 commits into from
Closed

Limit repository size #7833

wants to merge 4 commits into from

Conversation

sapk
Copy link
Member

@sapk sapk commented Aug 12, 2019

The goal of this PR is to define a repo limit size. I think org and user level restriction could come later.

TODO:

  • Calculate Push Size (I still have some corner-case to test mostly to not block deletion and some force push)
  • Edit max repo size
  • Enforce repo size
  • Add tests
  • Validate LFS LFS object are not calculated in .Size repo attr so I will skip LFS checks.
Screenshots:

image
image
image
image
image
image

TOFIX :

Deletion of file from UI trigger 500 when repo is over. -> TODO catch this specific error. 2019/08/16 05:23:58 ...uters/repo/editor.go:432:DeleteFilePost() [E] DeleteRepoFile: git push: remote: Gitea: new repo size is over limitation 10000 To /home/sapk/go/src/code.gitea.io/gitea/data/repositories/sapk/test.git ! [remote rejected] d9629b41f9c58da756cf806aabf5811b1ff45b50 -> master (pre-receive hook declined) error: impossible de pousser des références vers '/home/sapk/go/src/code.gitea.io/gitea/data/repositories/sapk/test.git'
Creation of branch from UI trigger 500 when repo is over. -> TODO catch this specific error. 2019/08/16 05:28:42 ...uters/repo/branch.go:287:CreateBranch() [E] CreateNewBranch: Push: exit status 1 - remote: Gitea: new repo size is over limitation 10000 To /home/sapk/go/src/code.gitea.io/gitea/data/repositories/sapk/test.git ! [remote rejected] test -> test (pre-receive hook declined) error: impossible de pousser des références vers '/home/sapk/go/src/code.gitea.io/gitea/data/repositories/sapk/test.git'

Related: #3658

@codecov-io
Copy link

codecov-io commented Aug 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@a8da0a0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7833   +/-   ##
=========================================
  Coverage          ?   41.41%           
=========================================
  Files             ?      479           
  Lines             ?    63960           
  Branches          ?        0           
=========================================
  Hits              ?    26490           
  Misses            ?    34019           
  Partials          ?     3451

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8da0a0...7cd8e02. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 12, 2019
@lafriks lafriks added the pr/wip This PR is not ready for review label Aug 13, 2019
@oscarlofwenhamn
Copy link
Contributor

Nice, this will be a good function. 👍

@sapk
Copy link
Member Author

sapk commented Aug 15, 2019

This PR is near ready it just miss the tests. You can give it a try.

@sapk sapk changed the title [WIP] Limit repo size Limit repository size Aug 15, 2019
@lafriks lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 15, 2019
models/repo.go Outdated Show resolved Hide resolved
modules/git/repo_tree.go Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Oct 22, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Oct 22, 2019
@oscarlofwenhamn
Copy link
Contributor

Is this dead?

@stale stale bot removed the issue/stale label Oct 23, 2019
@sapk
Copy link
Member Author

sapk commented Oct 24, 2019

@oscarlofwenhamn I haevn't really time go into it to fix the few nits. I will maybe split this PR in two: display (done), and enforce (need more work).

@oscarlofwenhamn
Copy link
Contributor

@sapk Fully understandable and relatable. Splitting it into two would be brilliant, that way we could get the finished functionality going and maybe someone could more easily help out on the enforce-PR to iron out the last creases without risking the finished stuff.

@stale
Copy link

stale bot commented Dec 23, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Dec 23, 2019
@6543
Copy link
Member

6543 commented Dec 24, 2019

@sapk ping

@stale stale bot removed the issue/stale label Dec 24, 2019
@sapk
Copy link
Member Author

sapk commented Dec 24, 2019

@6543 I worked first on splitting up a PR related to attachement security. I will do this one when the two last part of the first are merged.

@stale
Copy link

stale bot commented Feb 22, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Feb 22, 2020
@sapk
Copy link
Member Author

sapk commented Feb 23, 2020

Yes stale bot I should refresh this one. 😄

@stale
Copy link

stale bot commented Apr 28, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 28, 2020
@sapk
Copy link
Member Author

sapk commented Nov 7, 2020

I rebased and cleaned this PR. It still need work and will try to finish it in the next days.

@lunny
Copy link
Member

lunny commented Nov 8, 2020

And should we also include releases binaries? And attachments of issues/comments?

@lafriks
Copy link
Member

lafriks commented Nov 8, 2020

We should split lfs size to different column before this as lfs size is calculated into size now :(

@sapk
Copy link
Member Author

sapk commented Nov 17, 2020

@lunny and @lafriks, If you are ok with it I will keep this one on what is included in the current size calculation and when all the checks are in place we could improve them by splitting and adding more details on every parts.

@codecov-io
Copy link

Codecov Report

Merging #7833 (733a70c) into master (0615b66) will decrease coverage by 0.78%.
The diff coverage is 47.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7833      +/-   ##
==========================================
- Coverage   42.19%   41.41%   -0.79%     
==========================================
  Files         697      479     -218     
  Lines       76563    63960   -12603     
==========================================
- Hits        32309    26490    -5819     
+ Misses      38956    34019    -4937     
+ Partials     5298     3451    -1847     
Impacted Files Coverage Δ
models/migrations/migrations.go 1.30% <ø> (-1.15%) ⬇️
models/migrations/v93.go 0.00% <0.00%> (ø)
modules/auth/repo_form.go 41.73% <ø> (+0.14%) ⬆️
routers/repo/setting.go 9.39% <13.33%> (-5.26%) ⬇️
modules/git/repo_tree.go 42.85% <57.14%> (+2.85%) ⬆️
routers/private/hook.go 48.84% <60.60%> (+16.55%) ⬆️
models/repo.go 48.69% <100.00%> (-7.47%) ⬇️
modules/git/repo.go 59.90% <100.00%> (+14.72%) ⬆️
modules/pull/merge.go 36.28% <100.00%> (ø)
routers/private/internal.go 20.75% <0.00%> (-67.71%) ⬇️
... and 777 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0615b66...7c31cfc. Read the comment docs.

@lunny
Copy link
Member

lunny commented Dec 13, 2020

CI failed and conflicted.

@lunny lunny modified the milestones: 1.14.0, 1.x.x Jan 27, 2021
@melroy89

This comment has been minimized.

@DmitryFrolovTri
Copy link
Contributor

@sapk Hi is this one still alive?

@DmitryFrolovTri
Copy link
Contributor

DmitryFrolovTri commented Nov 8, 2022

Ok guys, I am planning to hire a developer to get this resolved hopefully I can get enough money, I estimate about 20h max on at 50$. Whatever remains would be allocated to another fix. Here is where I am trying to get the needed money:
https://opencollective.com/oss-code/projects/gitea-limit-repository-size-old

@techknowlogick
Copy link
Member

Thanks @DmitryFrolovTri, I emailed you, but I will also say here I'm having trouble backing this collective, but once resolved there will be some additional funds in that bounty.

cc: @kdumontnu who has supported bounties in the past who may also find this beneficial.

Should this bounty be funded I will also set aside some funds for @sapk for his work on this PR so far.

@kdumontnu
Copy link
Contributor

kdumontnu commented Nov 8, 2022

Thanks @DmitryFrolovTri, I emailed you, but I will also say here I'm having trouble backing this collective, but once resolved there will be some additional funds in that bounty.

cc: @kdumontnu who has supported bounties in the past who may also find this beneficial.

Should this bounty be funded I will also set aside some funds for @sapk for his work on this PR so far.

takemymoney

Yeah, I can probably come up with something.

Just not sure about using Open Collective for bounties. I've used bountysource in the past (which I don't love...). My question is just if this doesn't hit the funding amount in OC and/or sits for years where the funds go. Once I figure that out consider me in for $250.

@DmitryFrolovTri
Copy link
Contributor

DmitryFrolovTri commented Nov 9, 2022

@kdumontnu well I think this conversation is not for this thread :) But my plan to use them on other gitea / opencollective items there is also some admin costs which might be needed - OC keeps track of all this.

@DmitryFrolovTri
Copy link
Contributor

@kdumontnu hi, seems some bank details were incorrect I fixed that. My plan (worst case) if we don't reach the goal donate back to gitea minus some processing/keeping costs

@sapk
Copy link
Member Author

sapk commented Nov 12, 2022

Feel free to take over.
I don't really have time to finish this in the neartime and for the bounty you can do as you like. I would be already pleased with this feature being available in Gitea.

@DmitryFrolovTri
Copy link
Contributor

@DmitryFrolovTri
Copy link
Contributor

@sapk please give access to your PR to contribute or add as contributor to the sapk fork so I could push to limit-repo-size branch.

@techknowlogick
Copy link
Member

closing in favour of #21820

@DmitryFrolovTri DmitryFrolovTri mentioned this pull request Nov 15, 2022
30 tasks
@lunny lunny removed this from the 1.x.x milestone Nov 25, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.