Split lfs size from repository size #22900

a1012112796 · 2023-02-14T04:04:09Z

releated to #21820

Split Size in repository table as two new colunms, one is GitSize for git size, the other is LFSSize for lfs data. still store full size in Size colunm.
Show full size on ui, but show each of them by a title; example:
Return full size in api response.

releated to go-gitea#21820 splite `Size` in repository table as two colunms, one is `Size` for git size, the other is `LFSSize` for lfs data. still show full size on ui, but show each of them by a `title`; still response full size in api response. Signed-off-by: a1012112796 <[email protected]>

Signed-off-by: a1012112796 <[email protected]>

kousu · 2023-02-14T04:24:30Z

Nice. I have a question: what if lfs.ENABLED = false? Will this display '0 KiB' everywhere or should it hide that?

lunny · 2023-02-14T05:28:12Z

~~Maybe we should move the setting to a seperate table, after #22500 merged~~

modules/templates/helper.go

a1012112796 · 2023-02-14T09:53:54Z

Maybe we should move the setting to a seperate table, after #22500 merged

yes, but I think both Size and LFSSize are status value, not config data.

- show title when the lfs size isn't zero - merge `Add64` and `Add` helper function Signed-off-by: a1012112796 <[email protected]>

Signed-off-by: a1012112796 <[email protected]>

models/repo/repo.go

templates/repo/settings/options.tmpl

templates/repo/sub_menu.tmpl

lunny · 2023-02-14T23:50:13Z

For a repository, there are many related data not only git data. There are still, lfs, package, database tables, attachments. Should we have another x columns to store them?

silverwind · 2023-02-15T10:48:58Z

For a repository, there are many related data not only git data. There are still, lfs, package, database tables, attachments. Should we have another x columns to store them?

I think the main size value displayed should be the repo size that you get with git clone, other sizes can be shown in the tooltip, but should not be added up.

I guess other sizes can be done in a separate PR.

Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: silverwind <[email protected]>

Signed-off-by: a1012112796 <[email protected]>

a1012112796 · 2023-02-15T11:23:10Z

For a repository, there are many related data not only git data. There are still, lfs, package, database tables, attachments. Should we have another x columns to store them?

Hmm, How about use a json struct like this ?

diff --git a/models/repo/repo.go b/models/repo/repo.go
index a06ae7269..e733afd76 100644
--- a/models/repo/repo.go
+++ b/models/repo/repo.go
@@ -112,6 +112,17 @@ const (
 	RepositoryBroken                                  // repository is in a permanently broken state
 )
 
+type RepositorySize struct {
+	GitSize         int64
+	LFSSize         int64
+	AttachmentsSize int64
+	// ....
+}
+
+func (s RepositorySize) Size() int64 {
+	return s.GitSize + s.LFSSize
+}
+
 // Repository represents a git repository.
 type Repository struct {
 	ID                  int64 `xorm:"pk autoincr"`
@@ -163,8 +174,7 @@ type Repository struct {
 	IsTemplate bool        `xorm:"INDEX NOT NULL DEFAULT false"`
 	TemplateID int64       `xorm:"INDEX"`
 	// the size of git repository directory itself, not include lfs/package/attachment size
-	Size                            int64              `xorm:"NOT NULL DEFAULT 0"`
-	LFSSize                         int64              `xorm:"NOT NULL DEFAULT 0"`
+	Size                            RepositorySize     `xorm:"JSON TEXT"`
 	CodeIndexerStatus               *RepoIndexerStatus `xorm:"-"`
 	StatsIndexerStatus              *RepoIndexerStatus `xorm:"-"`
 	IsFsckEnabled                   bool               `xorm:"NOT NULL DEFAULT true"`

``

a1012112796 · 2023-02-15T11:27:30Z

other sizes can be shown in the tooltip

Hmm, I think show them on setting page to let repo owner know it is enough.

lunny · 2023-02-15T11:41:49Z

For a repository, there are many related data not only git data. There are still, lfs, package, database tables, attachments. Should we have another x columns to store them?

Hmm, How about use a json struct like this ?

diff --git a/models/repo/repo.go b/models/repo/repo.go
index a06ae7269..e733afd76 100644
--- a/models/repo/repo.go
+++ b/models/repo/repo.go
@@ -112,6 +112,17 @@ const (
 	RepositoryBroken                                  // repository is in a permanently broken state
 )
 
+type RepositorySize struct {
+	GitSize         int64
+	LFSSize         int64
+	AttachmentsSize int64
+	// ....
+}
+
+func (s RepositorySize) Size() int64 {
+	return s.GitSize + s.LFSSize
+}
+
 // Repository represents a git repository.
 type Repository struct {
 	ID                  int64 `xorm:"pk autoincr"`
@@ -163,8 +174,7 @@ type Repository struct {
 	IsTemplate bool        `xorm:"INDEX NOT NULL DEFAULT false"`
 	TemplateID int64       `xorm:"INDEX"`
 	// the size of git repository directory itself, not include lfs/package/attachment size
-	Size                            int64              `xorm:"NOT NULL DEFAULT 0"`
-	LFSSize                         int64              `xorm:"NOT NULL DEFAULT 0"`
+	Size                            RepositorySize     `xorm:"JSON TEXT"`
 	CodeIndexerStatus               *RepoIndexerStatus `xorm:"-"`
 	StatsIndexerStatus              *RepoIndexerStatus `xorm:"-"`
 	IsFsckEnabled                   bool               `xorm:"NOT NULL DEFAULT true"`

``

Or we can let Size as the total size and store details in this new column of size detail.

a1012112796 · 2023-02-15T11:51:21Z

Or we can let Size as the total size and store details in this new column of size detail.

then the Size (total size) looks useless, why store a data which can be sumed quickly in database?

lunny · 2023-02-15T13:38:57Z

Or we can let Size as the total size and store details in this new column of size detail.

then the Size (total size) looks useless, why store a data which can be sumed quickly in database?

I don't think so. If we need to filter with size, we need that column as index.

Signed-off-by: a1012112796 <[email protected]>

a1012112796 · 2023-02-17T10:12:31Z

Or we can let Size as the total size and store details in this new column of size detail.

then the Size (total size) looks useless, why store a data which can be sumed quickly in database?

I don't think so. If we need to filter with size, we need that column as index.

@lunny done, how about this now? 0c047b3

models/repo/update.go

DmitryFrolovTri · 2023-06-20T14:25:48Z

@a1012112796 please also update the description of the PR - we have not only split the Size into git size and lfs size in the table, we have also done so in the UI for administrator templates/admin/repo/list.tmpl and probably move the screenshot from #22900 (comment) to the PR description

silverwind · 2023-06-20T18:38:10Z

@a1012112796 decided that it is faster to do the change. PR into your branch: a1012112796#9 Split Size in repository table as two columns, one is Size for git size, the other is LFSSize for lfs data. in the Administrative UI. Feel free to accept / deny or rework to your liking. Notable this it is uncoditional, i.e. if LFS_START_SERVER=false, it would still show the column, which is probably acceptable, but not ideal. For simplicity and as LFS is no longer rare I think should be ok. What do you think?

We can also do a tooltip in the admin interface if you fancy (I think). But since size limit would be on this screen and in future LFS limit probably like that should be fine, also I suppose admin does care where his data is stored.

Can you check with longer owner and repo names? These admin tables are already bursting on their width, so I'm not too fond of adding another column.

lunny · 2023-06-21T01:05:16Z

@a1012112796 decided that it is faster to do the change. PR into your branch: a1012112796#9 Split Size in repository table as two columns, one is Size for git size, the other is LFSSize for lfs data. in the Administrative UI. Feel free to accept / deny or rework to your liking. Notable this it is uncoditional, i.e. if LFS_START_SERVER=false, it would still show the column, which is probably acceptable, but not ideal. For simplicity and as LFS is no longer rare I think should be ok. What do you think?

We can also do a tooltip in the admin interface if you fancy (I think). But since size limit would be on this screen and in future LFS limit probably like that should be fine, also I suppose admin does care where his data is stored.

Can you check with longer owner and repo names? These admin tables are already bursting on their width, so I'm not too fond of adding another column.

Group Watchers,Stars, Forks, Issues as Watchers / Stars / Forks / Issues and Group Size, LFS Size as Size / LFS Size.

DmitryFrolovTri · 2023-06-21T17:09:20Z

@a1012112796 decided that it is faster to do the change. PR into your branch: a1012112796#9 Split Size in repository table as two columns, one is Size for git size, the other is LFSSize for lfs data. in the Administrative UI. Feel free to accept / deny or rework to your liking. Notable this it is uncoditional, i.e. if LFS_START_SERVER=false, it would still show the column, which is probably acceptable, but not ideal. For simplicity and as LFS is no longer rare I think should be ok. What do you think?

We can also do a tooltip in the admin interface if you fancy (I think). But since size limit would be on this screen and in future LFS limit probably like that should be fine, also I suppose admin does care where his data is stored.

Can you check with longer owner and repo names? These admin tables are already bursting on their width, so I'm not too fond of adding another column.

Group Watchers,Stars, Forks, Issues as Watchers / Stars / Forks / Issues and Group Size, LFS Size as Size / LFS Size.

There are sorts. so may be we group just Size and LFS Size

silverwind · 2023-06-21T19:09:03Z

I guess we can reduce padding a bit, add this to base.css after rule at line 870:

.ui.table > thead > tr > th,
.ui.table > tbody > tr > td,
.ui.table > tr > td {
  padding-left: .5em;
  padding-right: .5em;
}

a1012112796 · 2023-06-25T12:11:52Z

test result, looks okay in my view.

or maybe limit max name length in ui like here?

silverwind · 2023-06-26T21:54:12Z

Ah, the table scrolls. Still, I would hide the column LFS_START_SERVER as already mentioned. Can you merge main branch once more please? It looks like another migration landed.

wxiaoguang · 2023-06-27T02:27:24Z

The "auto-ellipsis" table is already supported by g-table-auto-ellipsis, see the "notice" admin page. (not a must and not related to this PR)

…ize_junupdate

DmitryFrolovTri · 2023-06-28T06:31:47Z

@a1012112796. Hi did a merge of latest upstream. a1012112796#12

…nupdate Zzc/dev/split lfs size junupdate

silverwind · 2023-06-28T18:20:16Z

So I guess this is LGTM. I don't think it's worth checking whether LFS is enabled because storage space could still be used when LFS is disabled. Regarding global table padding reduction, I will follow up with a separate PR.

silverwind · 2023-06-28T21:04:49Z

FYI, #25568 will reduce table padding which will help here a bit.

DmitryFrolovTri · 2023-06-29T07:28:10Z

Great! moving on to the other PR then!

DmitryFrolovTri · 2023-06-29T07:48:58Z

@a1012112796 thank you!

a1012112796 added the type/enhancement An improvement of existing functionality label Feb 14, 2023

a1012112796 force-pushed the zzc/dev/split_lfs_size branch from 910dd5b to b149388 Compare February 14, 2023 04:05

fix lint

63fe364

Signed-off-by: a1012112796 <[email protected]>

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 14, 2023

wxiaoguang reviewed Feb 14, 2023

View reviewed changes

modules/templates/helper.go Outdated Show resolved Hide resolved

a1012112796 added 2 commits February 14, 2023 10:17

apply suggestions from code review

3df76e7

- show title when the lfs size isn't zero - merge `Add64` and `Add` helper function Signed-off-by: a1012112796 <[email protected]>

adapt return type in Add helper function

997ce53

Signed-off-by: a1012112796 <[email protected]>

lunny reviewed Feb 14, 2023

View reviewed changes

models/repo/repo.go Show resolved Hide resolved

lunny mentioned this pull request Feb 14, 2023

Limit repo size #21820

Open

30 tasks

silverwind reviewed Feb 14, 2023

View reviewed changes

templates/repo/settings/options.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Feb 14, 2023

View reviewed changes

templates/repo/sub_menu.tmpl Outdated Show resolved Hide resolved

a1012112796 and others added 3 commits February 15, 2023 19:00

Apply suggestions from code review

292922d

Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: silverwind <[email protected]>

Merge branch 'main' into zzc/dev/split_lfs_size

1dfd604

fix lint

a5fb15e

Signed-off-by: a1012112796 <[email protected]>

a1012112796 added 3 commits February 17, 2023 09:01

Merge branch 'main' into zzc/dev/split_lfs_size

ede9146

use repo size detail

0c047b3

Signed-off-by: a1012112796 <[email protected]>

revert changs about add

65b7183

Signed-off-by: a1012112796 <[email protected]>

lunny reviewed Feb 18, 2023

View reviewed changes

models/repo/update.go Outdated Show resolved Hide resolved

a1012112796 added 2 commits June 25, 2023 00:34

Merge branch 'main' into zzc/dev/split_lfs_size

53c7e2e

Merge branch 'main' into zzc/dev/split_lfs_size

651dbfd

Merge remote-tracking branch 'upstream/main' into zzc/dev/split_lfs_s…

2cc9bf3

…ize_junupdate

Merge pull request #12 from DmitryFrolovTri/zzc/dev/split_lfs_size_ju…

1a63a59

…nupdate Zzc/dev/split lfs size junupdate

silverwind approved these changes Jun 28, 2023

View reviewed changes

silverwind added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 28, 2023

silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 28, 2023

GiteaBot added 2 commits June 28, 2023 17:29

Merge branch 'main' into zzc/dev/split_lfs_size

f077e0a

Merge branch 'main' into zzc/dev/split_lfs_size

8659688

silverwind changed the title ~~split lfs size from repository size~~ Split lfs size from repository size Jun 28, 2023

silverwind enabled auto-merge (squash) June 28, 2023 21:33

Merge branch 'main' into zzc/dev/split_lfs_size

4d51de1

silverwind merged commit 4aba8a6 into go-gitea:main Jun 28, 2023

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 28, 2023

a1012112796 deleted the zzc/dev/split_lfs_size branch June 29, 2023 05:06

go-gitea locked as resolved and limited conversation to collaborators Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split lfs size from repository size #22900

Split lfs size from repository size #22900

a1012112796 commented Feb 14, 2023 •

edited

Loading

kousu commented Feb 14, 2023

lunny commented Feb 14, 2023 •

edited

Loading

a1012112796 commented Feb 14, 2023

lunny commented Feb 14, 2023

silverwind commented Feb 15, 2023 •

edited

Loading

a1012112796 commented Feb 15, 2023

a1012112796 commented Feb 15, 2023

lunny commented Feb 15, 2023

a1012112796 commented Feb 15, 2023

lunny commented Feb 15, 2023

a1012112796 commented Feb 17, 2023

DmitryFrolovTri commented Jun 20, 2023

silverwind commented Jun 20, 2023 •

edited

Loading

lunny commented Jun 21, 2023

DmitryFrolovTri commented Jun 21, 2023

silverwind commented Jun 21, 2023

a1012112796 commented Jun 25, 2023

silverwind commented Jun 26, 2023 •

edited

Loading

wxiaoguang commented Jun 27, 2023 •

edited

Loading

DmitryFrolovTri commented Jun 28, 2023

silverwind commented Jun 28, 2023

silverwind commented Jun 28, 2023

DmitryFrolovTri commented Jun 29, 2023

DmitryFrolovTri commented Jun 29, 2023

Split lfs size from repository size #22900

Split lfs size from repository size #22900

Conversation

a1012112796 commented Feb 14, 2023 • edited Loading

kousu commented Feb 14, 2023

lunny commented Feb 14, 2023 • edited Loading

a1012112796 commented Feb 14, 2023

lunny commented Feb 14, 2023

silverwind commented Feb 15, 2023 • edited Loading

a1012112796 commented Feb 15, 2023

a1012112796 commented Feb 15, 2023

lunny commented Feb 15, 2023

a1012112796 commented Feb 15, 2023

lunny commented Feb 15, 2023

a1012112796 commented Feb 17, 2023

DmitryFrolovTri commented Jun 20, 2023

silverwind commented Jun 20, 2023 • edited Loading

lunny commented Jun 21, 2023

DmitryFrolovTri commented Jun 21, 2023

silverwind commented Jun 21, 2023

a1012112796 commented Jun 25, 2023

silverwind commented Jun 26, 2023 • edited Loading

wxiaoguang commented Jun 27, 2023 • edited Loading

DmitryFrolovTri commented Jun 28, 2023

silverwind commented Jun 28, 2023

silverwind commented Jun 28, 2023

DmitryFrolovTri commented Jun 29, 2023

DmitryFrolovTri commented Jun 29, 2023

a1012112796 commented Feb 14, 2023 •

edited

Loading

lunny commented Feb 14, 2023 •

edited

Loading

silverwind commented Feb 15, 2023 •

edited

Loading

silverwind commented Jun 20, 2023 •

edited

Loading

silverwind commented Jun 26, 2023 •

edited

Loading

wxiaoguang commented Jun 27, 2023 •

edited

Loading