Feature size repo in admin panel #39

DblK · 2016-11-03T13:11:08Z

Equivalent of my PR #3631 on gogs

strk · 2016-11-03T17:04:55Z

models/repo.go

+	if err = sess.Begin(); err != nil {
+		return nil
+	}
+	if _, err = sess.Exec("UPDATE `repository` SET repo_size=? WHERE name=?", repoSize, repoName); err != nil {


Had you tested this with PostgreSQL backend ?

No I didn't. I will need to make another install to let this work. I used Sqllite. If someone can give a try before me just do it ;).

Please avoid plain SQL queries and use XORM.

I have done the same thing as previous SQL (See L1372).
I will do it in XORM but only mine.

A review of SQL queries need to be done.

strk · 2016-11-03T17:16:42Z

At the moment I'm unable to build the code, so cannot test.

tboerger · 2016-11-03T18:59:45Z

models/repo.go

+	if err = sess.Begin(); err != nil {
+		return nil
+	}
+	if _, err = sess.Exec("UPDATE `repository` SET repo_size=? WHERE name=?", repoSize, repoName); err != nil {


Please avoid plain SQL queries and use XORM.

codecov-io · 2016-11-03T20:56:03Z

Current coverage is 2.15% (diff: 0.00%)

Merging #39 into master will decrease coverage by 0.02%

@@            master       #39   diff @@
========================================
  Files           31        31          
  Lines         7508      7593    +85   
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits           164       164          
- Misses        7327      7412    +85   
  Partials        17        17

Powered by Codecov. Last update fc55182...74cbcef

DblK · 2016-11-04T09:43:30Z

Changes made

strk · 2016-11-04T10:01:55Z

Applying this change and going to my Admin panel I see all 0B as the repositories size, does this miss an automatic migration ?

bkcsoft · 2016-11-04T10:43:46Z

models/repo.go

@@ -1666,6 +1693,125 @@ func GitGcRepos() error {
 		})
 }

+// Maybe move this to gogits/git-module


Make a separate issue for that (when this is merged) instead of adding useless comments in the source-code 😉

I just copy/paste the comments because in the same file there is also another block to move.
Without comments and depending on the time for the PR to get merge, I will probably forget this (because I haven't only one PR)

strk · 2016-11-04T10:46:39Z

I actually like comments in the source-code, so that people reading the code can get inspired to help by doing what the commentor left to the reader as an excercise :)

DblK · 2016-11-04T11:20:06Z

@strk The size will be updated in the next commit.
I could make another PR to make a migration script (even if I don't know how to do it).

strk · 2016-11-04T12:27:03Z

I don't know how to do migrations either, and I do realize on next commit the size would be updated. But I think its better to figure that out that to keep getting bug reports about those sizes being at 0

DblK · 2016-11-04T12:31:34Z

An easy way, could be to provide in the admin panel a button to force compute of all 0 sized repository.
What do you think about that?

strk · 2016-11-04T12:37:36Z

If I'm not mistaken there's also a periodic "repository checker" routine (not sure what it does, maybe GC) -- how about adding the count updates there ? Or, I've noticed other caches are being updated at startup (count if issues on which labels are applied) - could be another place, even less intrusive.

DblK · 2016-11-04T12:44:06Z

I didn't want to put in a periodic function because there are only two cases when we should update:

When an Update occurs
To compute if the data isn't set (migration)

The less intrusive could be at startup, but only watching repositories with a size of 0 to not spend too much time at startup (if you have a bunch of repos, it should be something that will lower the startup boot sequence too much).

strk · 2016-11-04T12:57:43Z

It's ok with me to only look at repos with size of 0 at startup, if migration is too hard to do. Maybe @lunny can give advice here about xorm migrations

it was addressed

strk · 2016-11-04T14:26:59Z

The new feature still needs automated tests

lunny · 2016-11-04T14:29:28Z

see https://github.com/go-gitea/gitea/tree/master/models/migrations to find some examples about migrations. You have to write a migration to set the default reposize value of the exsit repos.

DblK · 2016-11-04T14:36:39Z

Thanks @lunny for the help. I will look to this to make a migration script.

@strk Can you point me to something that show how to make tests for gitea?

strk · 2016-11-04T15:46:11Z

I actually never made a test in Gogs/Gitea,
but you can get a list of existing tests with something like:

find . -name "*_test.go"

It looks like all of them use github.com/smartystreets/goconvey/convey

metalmatze · 2016-11-04T19:37:38Z

IMHO we should get rid of goconvey with time. Thoughts?

strk · 2016-11-04T20:30:05Z

On Fri, Nov 04, 2016 at 12:37:38PM -0700, Matthias Loibl wrote:

IMHO we should get rid of goconvey with time. Thoughts?

I've no experiences with different testing frameworks,
but I do think tests should be there.

lunny · 2016-11-05T02:39:50Z

@metalmatze We need to build a new test suites before we remove goconvey. This is a plan.

thibaultmeyer · 2016-11-13T09:44:25Z

models/repo.go

+const STAT_GARBAGE = "garbage: "
+const STAT_SIZEGARBAGE = "size-garbage: "
+
+func GitRepoSize(repoPath string) (*CountObject, error) {


This method should be on the git module, not here

thibaultmeyer · 2016-11-13T09:44:47Z

Why put it on database ? It could be easily become out-dated, by exemple, if server run periodically housekeeping operation via CRON outside from Gitea.

Why not simply use the command git count-objects directly ?

thibaultmeyer · 2016-11-13T09:55:11Z

models/repo.go

+
+func GitRepoSize(repoPath string) (*CountObject, error) {
+	var cmd *exec.Cmd
+	cmd = exec.Command("git", "count-objects", "-v")


Please add the flag --human-readable

This function should really in git repository.

bkcsoft · 2016-12-01T09:03:55Z

Why put it on database ? It could be easily become out-dated, by exemple, if server run periodically housekeeping operation via CRON outside from Gitea.

Don't do this 😆 Repos should be owned by Gitea...

Why not simply use the command git count-objects directly ?

Slow as fsck... at least until someome implements a cache-layer 😉

lunny · 2016-12-25T09:46:21Z

Please rebase and we can continue to review this.

tboerger · 2016-12-29T10:29:34Z

@DblK any update? Need to resolve the conflicts.

Bwko · 2017-01-11T19:25:16Z

@DblK any updates on this?

tboerger · 2017-01-24T19:00:36Z

Looks like @DblK don't have time to come back to this. Maybe somebody else wants to takeover and finish this PR?

lunny · 2017-02-05T14:01:37Z

So let's move it to v1.2

tboerger · 2017-02-26T23:00:19Z

Maybe we should just takeover this pr?

cez81 · 2017-03-31T13:21:41Z

May I take this one?

lunny · 2017-04-01T01:02:42Z

@cez81 please do it.

appleboy · 2017-04-11T13:30:56Z

Closed via #1482

Signed-off-by: Dennis Keitzel <[email protected]>

DblK changed the title ~~Feature size repo~~ Feature size repo in admin panel Nov 3, 2016

strk reviewed Nov 3, 2016

View reviewed changes

tboerger added this to the 1.0.0 milestone Nov 3, 2016

tboerger added the type/enhancement An improvement of existing functionality label Nov 3, 2016

tboerger previously requested changes Nov 3, 2016

View reviewed changes

tboerger added type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/enhancement An improvement of existing functionality labels Nov 3, 2016

Implementation of the feature to view repository size in admin panel

74cbcef

bkcsoft reviewed Nov 4, 2016

View reviewed changes

strk modified the milestones: 1.1.0, 1.0.0 Nov 6, 2016

thibaultmeyer suggested changes Nov 13, 2016

View reviewed changes

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 1, 2016

tboerger added pr/wip This PR is not ready for review issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail and removed pr/wip This PR is not ready for review labels Dec 29, 2016

lunny added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Jan 25, 2017

lunny modified the milestones: 1.2.0, 1.1.0 Feb 5, 2017

cez81 mentioned this pull request Apr 3, 2017

Add GetRepoSize implemented using count-objects go-gitea/git#40

Merged

cez81 mentioned this pull request Apr 10, 2017

Repo size in admin panel #1482

Merged

appleboy closed this Apr 11, 2017

lunny removed this from the 1.2.0 milestone Apr 11, 2017

lunny pushed a commit to lunny/gitea that referenced this pull request Feb 7, 2019

Return empty list of branches if repository is bare. (go-gitea#39)

43e5216

Signed-off-by: Dennis Keitzel <[email protected]>

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Uh oh!

Feature size repo in admin panel #39

Feature size repo in admin panel #39

Uh oh!

Conversation

DblK commented Nov 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

strk commented Nov 3, 2016 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 2.15% (diff: 0.00%)

Uh oh!

DblK commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

strk commented Nov 4, 2016 via email

Uh oh!

DblK commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016

Uh oh!

DblK commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016 via email

Uh oh!

DblK commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016 via email

Uh oh!

strk commented Nov 4, 2016

Uh oh!

lunny commented Nov 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DblK commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016

Uh oh!

metalmatze commented Nov 4, 2016

Uh oh!

strk commented Nov 4, 2016

Uh oh!

lunny commented Nov 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thibaultmeyer commented Nov 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkcsoft commented Dec 1, 2016

Uh oh!

lunny commented Dec 25, 2016

Uh oh!

tboerger commented Dec 29, 2016

Uh oh!

Bwko commented Jan 11, 2017

Uh oh!

tboerger commented Jan 24, 2017

Uh oh!

lunny commented Feb 5, 2017

codecov-io commented Nov 3, 2016 •

edited

Loading

lunny commented Nov 4, 2016 •

edited

Loading

thibaultmeyer commented Nov 13, 2016 •

edited

Loading