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

add parameters for PostgreSQL #16641

Merged
merged 1 commit into from
Nov 30, 2022
Merged

add parameters for PostgreSQL #16641

merged 1 commit into from
Nov 30, 2022

Conversation

sayaoailun
Copy link
Contributor

add conn_max_lifetime and conn_max_idle_time

@sayaoailun sayaoailun force-pushed the main branch 2 times, most recently from 322f594 to ef12b04 Compare April 3, 2022 13:21
@Vad1mo Vad1mo requested review from stonezdj and heww and removed request for stonezdj April 4, 2022 22:04
@codecov
Copy link

codecov bot commented Apr 12, 2022

Codecov Report

Merging #16641 (444f6ce) into main (402363d) will increase coverage by 22.18%.
The diff coverage is 71.92%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main   #16641       +/-   ##
===========================================
+ Coverage   44.25%   66.43%   +22.18%     
===========================================
  Files         244     1012      +768     
  Lines       13389   108540    +95151     
  Branches     2678     2675        -3     
===========================================
+ Hits         5925    72110    +66185     
- Misses       7177    32475    +25298     
- Partials      287     3955     +3668     
Flag Coverage Δ
unittests 66.43% <71.92%> (+22.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/lib/config/systemconfig.go 49.27% <0.00%> (ø)
src/common/dao/pgsql.go 68.04% <77.77%> (ø)
src/lib/config/metadata/value.go 55.26% <83.33%> (ø)
src/common/dao/base.go 58.20% <100.00%> (ø)
src/lib/config/metadata/type.go 75.60% <100.00%> (ø)
src/pkg/config/manager.go 49.13% <100.00%> (ø)
...d/worker-card/donut-chart/donut-chart.component.ts 48.57% <0.00%> (-11.43%) ⬇️
src/server/v2.0/handler/model/jobservice.go 0.00% <0.00%> (ø)
src/lib/orm/tx.go 84.61% <0.00%> (ø)
src/pkg/reg/adapter/gitlab/client.go 47.16% <0.00%> (ø)
... and 767 more

@MinerYang MinerYang requested review from Vad1mo and chlins April 18, 2022 09:01
@wy65701436 wy65701436 added release-note/enhancement Label to mark PR to be added under release notes as enhancement candidate/2.7.0 labels Jul 30, 2022
@github-actions
Copy link

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Sep 29, 2022
@sayaoailun
Copy link
Contributor Author

/remove Stale

@wy65701436
Copy link
Contributor

@chlins we need to validate it before merging.

@chlins
Copy link
Member

chlins commented Nov 15, 2022

@sayaoailun Hi, please rebase your pr with main branch, thanks!

@sayaoailun
Copy link
Contributor Author

@sayaoailun Hi, please rebase your pr with main branch, thanks!

@chlins Done.

@chlins
Copy link
Member

chlins commented Nov 29, 2022

@sayaoailun Hi, thanks, but recently we've merged the beego upgrade to the main which may have conflicts and impacts to your pr, so maybe you need to rebase again and then revise the code because beego orm update have some changes in this part.

@sayaoailun
Copy link
Contributor Author

@sayaoailun Hi, thanks, but recently we've merged the beego upgrade to the main which may have conflicts and impacts to your pr, so maybe you need to rebase again and then revise the code because beego orm update have some changes in this part.

@chlins Done.

@chlins
Copy link
Member

chlins commented Nov 29, 2022

@sayaoailun Thanks your work! I'll merge it after verification.

@@ -96,6 +100,10 @@ func (p *pgsql) Register(alias ...string) error {
return err
}

db, _ := orm.GetDB(an)
db.SetConnMaxLifetime(p.connMaxLifetime)
Copy link
Member

Choose a reason for hiding this comment

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

@sayaoailun Please review the code(line 98), there are some difference after beego upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chlins Fixed.

return err
}

db, _ := orm.GetDB(an)
Copy link
Member

Choose a reason for hiding this comment

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

We'd better check error here to avoid db is nil then will panic in the next line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chlins Fixed.

@sayaoailun sayaoailun force-pushed the main branch 2 times, most recently from f8d506d to 1561be7 Compare November 29, 2022 06:00
src/cmd/exporter/main.go Outdated Show resolved Hide resolved
Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins
Copy link
Member

chlins commented Nov 30, 2022

@sayaoailun Please check the CI action, the UTTEST failed.

@sayaoailun
Copy link
Contributor Author

@sayaoailun Please check the CI action, the UTTEST failed.

@chlins Fixed.

@chlins chlins merged commit cb11540 into goharbor:main Nov 30, 2022
tpoxa pushed a commit to container-registry/harbor-next that referenced this pull request Feb 8, 2023
Signed-off-by: sayaoailun <[email protected]>
Signed-off-by: Maksym <[email protected]>
mcsage pushed a commit to mcsage/harbor that referenced this pull request Feb 16, 2023
Signed-off-by: sayaoailun <[email protected]>
Signed-off-by: Stephan Hohn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/enhancement Label to mark PR to be added under release notes as enhancement target/2.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants