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

Remove redundant call to UpdateRepoStats during migration #18591

Merged
merged 7 commits into from
Feb 7, 2022

Conversation

singuliere
Copy link
Contributor

There is no need to call UpdateRepoStats in the InsertIssues and
InsertPullRequests function. They are only called during migration by
the CreateIssues and CreateReviews methods of the gitea uploader.

The UpdateRepoStats function will be called by the Finish method of
the gitea uploader after all reviews and issues are inserted. Calling
it before is therefore redundant and the associated SQL requests are
not cheap.

The statistics tests done after inserting an issue or a pull request
are also removed. They predate the implementation of UpdateRepoStats,
back when the calculation of the statistics was an integral part of
the migration function. The UpdateRepoStats is now tested
independantly and these tests are no longer necessary.

Signed-off-by: singuliere [email protected]

@singuliere singuliere added type/enhancement An improvement of existing functionality performance/speed performance issues with slow downs labels Feb 4, 2022
@singuliere singuliere added this to the 1.17.0 milestone Feb 4, 2022
@singuliere singuliere marked this pull request as draft February 4, 2022 08:07
There is no need to call UpdateRepoStats in the InsertIssues and
InsertPullRequests function. They are only called during migration by
the CreateIssues and CreateReviews methods of the gitea uploader.

The UpdateRepoStats function will be called by the Finish method of
the gitea uploader after all reviews and issues are inserted. Calling
it before is therefore redundant and the associated SQL requests are
not cheap.

The statistics tests done after inserting an issue or a pull request
are also removed. They predate the implementation of UpdateRepoStats,
back when the calculation of the statistics was an integral part of
the migration function. The UpdateRepoStats is now tested
independantly and these tests are no longer necessary.

Signed-off-by: singuliere <[email protected]>
@singuliere singuliere marked this pull request as ready for review February 4, 2022 08:39
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 4, 2022
@singuliere singuliere added the topic/repo-migration Migrate repos from other platforms to Gitea, or from Gitea to them label Feb 4, 2022
Comment on lines +35 to +37
func assertCreateIssues(t *testing.T, isPull bool) {
assert.NoError(t, unittest.PrepareTestDatabase())
reponame := "repo1"
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is now redundant, we might need to add tests later for other repo's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. There is another pull request adding more testing for repo dump/restore and preparing the ground for adding verification for other repositories.

@GiteaBot GiteaBot 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 Feb 5, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2022

Codecov Report

Merging #18591 (b22035d) into main (80048c0) will increase coverage by 0.05%.
The diff coverage is 48.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18591      +/-   ##
==========================================
+ Coverage   46.25%   46.30%   +0.05%     
==========================================
  Files         842      846       +4     
  Lines      121180   121195      +15     
==========================================
+ Hits        56052    56120      +68     
+ Misses      58321    58270      -51     
+ Partials     6807     6805       -2     
Impacted Files Coverage Δ
models/commit.go 62.50% <0.00%> (ø)
models/migrate.go 40.00% <ø> (+0.73%) ⬆️
models/release.go 49.45% <0.00%> (-1.49%) ⬇️
models/review.go 50.29% <0.00%> (-0.60%) ⬇️
models/user/external_login_user.go 28.57% <ø> (ø)
modules/git/notes_nogogit.go 66.66% <0.00%> (-2.19%) ⬇️
modules/gitgraph/graph_models.go 80.85% <0.00%> (ø)
modules/migration/pullrequest.go 0.00% <0.00%> (ø)
modules/migration/release.go 0.00% <0.00%> (ø)
modules/migration/review.go 0.00% <0.00%> (ø)
... and 34 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 9419dd2...b22035d. Read the comment docs.

@techknowlogick techknowlogick merged commit 3a91f84 into go-gitea:main Feb 7, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 8, 2022
* giteaofficial/main: (28 commits)
  Added auto-save whitespace behavior if it changed manually (go-gitea#15566)
  Support custom ACME provider (go-gitea#18340)
  Refactor i18n, use Locale to provide i18n/translation related functions (go-gitea#18648)
  Only request write when necessary (go-gitea#18657)
  [skip ci] Updated translations via Crowdin
  Add separate SSH_USER config option (go-gitea#17584)
  Be more lenient with label colors (go-gitea#17752)
  remove redundant call to UpdateRepoStats during migration (go-gitea#18591)
  more repo dump/restore tests, including pull requests (go-gitea#18621)
  No longer show the db-downgrade SQL in production (go-gitea#18653)
  Fix the missing i18n key for update checker (go-gitea#18646)
  Update gitea-vet (go-gitea#18640)
  Future proof for 1.18 (go-gitea#18644)
  Add `contrib/upgrade.sh` (go-gitea#18286)
  If rendering has failed due to a net.OpError stop rendering (go-gitea#18642)
  Delete old git.NewCommand() and use it as git.NewCommandContext() (go-gitea#18552)
  Update JS dependencies (go-gitea#18636)
  fix commits_list_small.tmpl (go-gitea#18641)
  Fix `make fmt` and `make fmt-check` (go-gitea#18633)
  Frontport of changelog for v1.16.1 (go-gitea#18615)
  ...
singuliere added a commit to singuliere/gitea that referenced this pull request Feb 17, 2022
…8591)

There is no need to call UpdateRepoStats in the InsertIssues and
InsertPullRequests function. They are only called during migration by
the CreateIssues and CreateReviews methods of the gitea uploader.

The UpdateRepoStats function will be called by the Finish method of
the gitea uploader after all reviews and issues are inserted. Calling
it before is therefore redundant and the associated SQL requests are
not cheap.

The statistics tests done after inserting an issue or a pull request
are also removed. They predate the implementation of UpdateRepoStats,
back when the calculation of the statistics was an integral part of
the migration function. The UpdateRepoStats is now tested
independantly and these tests are no longer necessary.

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

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@lunny lunny added the backport/done All backports for this PR have been created label Feb 17, 2022
zeripath added a commit that referenced this pull request Feb 17, 2022
…18794)

There is no need to call UpdateRepoStats in the InsertIssues and
InsertPullRequests function. They are only called during migration by
the CreateIssues and CreateReviews methods of the gitea uploader.

The UpdateRepoStats function will be called by the Finish method of
the gitea uploader after all reviews and issues are inserted. Calling
it before is therefore redundant and the associated SQL requests are
not cheap.

The statistics tests done after inserting an issue or a pull request
are also removed. They predate the implementation of UpdateRepoStats,
back when the calculation of the statistics was an integral part of
the migration function. The UpdateRepoStats is now tested
independantly and these tests are no longer necessary.

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

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@zeripath zeripath changed the title remove redundant call to UpdateRepoStats during migration Remove redundant call to UpdateRepoStats during migration Feb 21, 2022
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
…8591)

There is no need to call UpdateRepoStats in the InsertIssues and
InsertPullRequests function. They are only called during migration by
the CreateIssues and CreateReviews methods of the gitea uploader.

The UpdateRepoStats function will be called by the Finish method of
the gitea uploader after all reviews and issues are inserted. Calling
it before is therefore redundant and the associated SQL requests are
not cheap.

The statistics tests done after inserting an issue or a pull request
are also removed. They predate the implementation of UpdateRepoStats,
back when the calculation of the statistics was an integral part of
the migration function. The UpdateRepoStats is now tested
independantly and these tests are no longer necessary.

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

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. performance/speed performance issues with slow downs topic/repo-migration Migrate repos from other platforms to Gitea, or from Gitea to them type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants