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.

Sign up for GitHub

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

Jump to bottom

Implement getParameter method for SQLite #2985

Merged
merged 16 commits into from Jul 11, 2023
Merged

Implement getParameter method for SQLite #2985

merged 16 commits into from Jul 11, 2023

Conversation

chilagrow
Copy link
Contributor

@chilagrow chilagrow commented Jul 5, 2023 •
edited

Description

Closes #2822.

Readiness checklist

  • I added/updated unit tests.
  • I added/updated integration/compatibility tests.
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@chilagrow chilagrow self-assigned this Jul 5, 2023
@chilagrow chilagrow added the code/chore Code maintenance improvements label Jul 5, 2023
@codecov
Copy link

codecov bot commented Jul 5, 2023 •
edited

Codecov Report

Merging #2985 (165b335) into main (db7fecf) will decrease coverage by 8.15%.
The diff coverage is 70.29%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2985      +/-   ##
==========================================
- Coverage   76.26%   68.11%   -8.15%     
==========================================
  Files         385      386       +1     
  Lines       21045    21056      +11     
==========================================
- Hits        16049    14342    -1707     
- Misses       4074     5765    +1691     
- Partials      922      949      +27
Impacted Files Coverage Δ
internal/handlers/hana/msg_getparameter.go 0.00% <0.00%> (ø)
internal/handlers/common/getparameter.go 70.40% <70.40%> (ø)
internal/handlers/pg/msg_getparameter.go 100.00% <100.00%> (+15.73%) ⬆️
internal/handlers/sqlite/msg_getparameter.go 100.00% <100.00%> (ø)

... and 62 files with indirect coverage changes

Flag Coverage Δ
integration 64.11% <70.29%> (-8.65%) ⬇️
mongodb 5.34% <0.00%> (-0.01%) ⬇️
pg 57.93% <69.30%> (-8.25%) ⬇️
shard-1 55.92% <0.00%> (+0.71%) ⬆️
shard-2 54.68% <70.29%> (-0.81%) ⬇️
shard-3 ?
sqlite 42.07% <69.30%> (-3.86%) ⬇️
unit 24.25% <0.00%> (-0.02%) ⬇️

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

@chilagrow chilagrow marked this pull request as ready for review July 6, 2023 03:35
@chilagrow chilagrow requested review from AlekSi and a team as code owners July 6, 2023 03:35
@chilagrow chilagrow requested review from rumyantseva, a team, quasilyte and noisersup July 6, 2023 03:35
@chilagrow chilagrow enabled auto-merge (squash) July 6, 2023 03:36
@chilagrow chilagrow changed the title Implement getParameters for authenticationMechanisms and featureCompatibilityVersion Support getParameters for authenticationMechanisms and featureCompatibilityVersion parameters Jul 6, 2023
@chilagrow
Copy link
Contributor Author 

$ task test-integration-sqlite TEST_RUN='TestCommandsAdministrationGetParameter|TestGetParameterCommandAuthenticationMechanisms'
task: [test-integration-sqlite] go test -count=1 -run='TestCommandsAdministrationGetParameter|TestGetParameterCommandAuthenticationMechanisms' -timeout=35m -race=true -tags=ferretdb_debug,ferretdb_tigris,ferretdb_hana -shuffle=on -coverpkg=../... -coverprofile=integration-sqlite.txt . -target-backend=ferretdb-sqlite -target-tls -compat-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem' -disable-filter-pushdown

ok      github.com/FerretDB/FerretDB/integration        0.366s  coverage: 40.0% of statements in ../...

rumyantseva
rumyantseva previously approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

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

LGTM

integration/commands_administration_test.go Show resolved Hide resolved
noisersup
noisersup previously approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@noisersup noisersup left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2023

@chilagrow this pull request has merge conflicts.

@mergify mergify bot added the conflict PRs that have merge conflicts label Jul 6, 2023
@chilagrow chilagrow dismissed stale reviews from noisersup and rumyantseva via 1a69b8e July 7, 2023 01:19
@mergify mergify bot removed the conflict PRs that have merge conflicts label Jul 7, 2023
@chilagrow
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jul 10, 2023

update

✅ Branch has been successfully updated

@chilagrow chilagrow requested a review from AlekSi July 10, 2023 12:34
AlekSi
AlekSi reviewed Jul 10, 2023
View reviewed changes
Copy link
Member

@AlekSi AlekSi left a comment

Choose a reason for hiding this comment

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

#2985 (review)

@chilagrow chilagrow dismissed stale reviews from noisersup and rumyantseva via 2291c54 July 11, 2023 02:00
@AlekSi AlekSi requested review from a team July 11, 2023 06:21
@chilagrow @AlekSi
Update internal/handlers/common/getparameter.go
3c6d1e9 
Co-authored-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com>
@chilagrow chilagrow requested a review from AlekSi July 11, 2023 06:30
@chilagrow chilagrow merged commit e834ee5 into FerretDB:main Jul 11, 2023
23 of 27 checks passed
@AlekSi AlekSi added this to the v1.6.0 milestone Jul 17, 2023
@AlekSi AlekSi added code/enhancement Some user-visible feature could work better and removed code/chore Code maintenance improvements labels Jul 18, 2023
@AlekSi AlekSi changed the title Support getParameters for authenticationMechanisms and featureCompatibilityVersion parameters Implement getParameter method for SQLite Jul 18, 2023
@AlekSi AlekSi added code/feature Some user-visible feature is not implemented yet and removed code/enhancement Some user-visible feature could work better labels Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlekSi AlekSi AlekSi approved these changes

@quasilyte quasilyte quasilyte approved these changes

@rumyantseva rumyantseva Awaiting requested review from rumyantseva

@noisersup noisersup Awaiting requested review from noisersup

Assignees

@chilagrow chilagrow

Labels
code/feature Some user-visible feature is not implemented yet
Projects
Current Engineering
Archived in project
Milestone
v1.6.0
Development

Successfully merging this pull request may close these issues.

Implement getParameter method
5 participants
@chilagrow @AlekSi @rumyantseva @quasilyte @noisersup