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

Allow filtering by name in ListDatabases and ListCollections #3851

Merged
merged 36 commits into from
Jan 4, 2024
Merged

Allow filtering by name in ListDatabases and ListCollections #3851

merged 36 commits into from
Jan 4, 2024

Conversation

sachinpuranik
Copy link
Contributor

@sachinpuranik sachinpuranik commented Dec 14, 2023 •
edited
Loading

Description

Refactor ListDatabases and ListCollections , to retrieve the DB names and collection names by name.
This is needed to combine multiple functionalities to one.
Note :
internal/backends/backend.go, internal/backends/database.go have a commented code. I consider it candidate for removal, since it's redundant. Kindly provide opinion.

Closes #3601

Readiness checklist

  • I added/updated unit tests (and they pass).
  • I added/updated integration/compatibility tests (and they pass).
  • 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), Milestone (Next), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@sachinpuranik sachinpuranik requested review from AlekSi and a team as code owners December 14, 2023 08:24
@CLAassistant
Copy link

CLAassistant commented Dec 14, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

sachinpuranik
sachinpuranik commented Dec 14, 2023
View reviewed changes
Copy link
Contributor Author

@sachinpuranik sachinpuranik left a comment

Choose a reason for hiding this comment

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

Left question post self review.

internal/backends/backend.go Outdated Show resolved Hide resolved
internal/backends/database.go Outdated Show resolved Hide resolved
@sachinpuranik sachinpuranik marked this pull request as draft December 14, 2023 09:12
@sachinpuranik sachinpuranik marked this pull request as ready for review December 14, 2023 09:14
@AlekSi AlekSi added the code/chore Code maintenance improvements label Dec 14, 2023
@AlekSi AlekSi added this to the Next milestone Dec 14, 2023
@AlekSi AlekSi self-assigned this Dec 14, 2023
@AlekSi AlekSi requested review from a team, henvic and noisersup December 14, 2023 09:30
@AlekSi AlekSi changed the title Fix for issue 3601, refactor ListDatabases and ListCollections Allow filtering by name in ListDatabases and ListCollections Dec 14, 2023
AlekSi
AlekSi reviewed Dec 14, 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.

#3851 (comment)

@codecov Codecov
Copy link

codecov bot commented Dec 14, 2023 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (c8f8c68) 74.98% compared to head (5e4eb89) 73.94%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3851      +/-   ##
==========================================
- Coverage   74.98%   73.94%   -1.05%     
==========================================
  Files         330      330              
  Lines       25676    25722      +46     
==========================================
- Hits        19254    19020     -234     
- Misses       5258     5569     +311     
+ Partials     1164     1133      -31
Files Coverage Δ
internal/backends/backend.go 93.54% <ø> (+6.45%) ⬆️
internal/backends/database.go 97.36% <ø> (+5.26%) ⬆️
internal/backends/decorators/oplog/collection.go 74.26% <100.00%> (-0.56%) ⬇️
internal/backends/postgresql/backend.go 68.88% <100.00%> (+4.78%) ⬆️
internal/backends/postgresql/database.go 74.80% <100.00%> (+3.75%) ⬆️
internal/backends/sqlite/backend.go 81.69% <100.00%> (+3.72%) ⬆️
internal/backends/sqlite/database.go 79.13% <100.00%> (+3.13%) ⬆️
internal/handler/msg_collstats.go 73.21% <100.00%> (-0.47%) ⬇️
internal/handler/msg_explain.go 60.90% <75.00%> (-0.70%) ⬇️
internal/handler/msg_find.go 80.10% <75.00%> (-0.21%) ⬇️
... and 1 more

... and 14 files with indirect coverage changes

Flag Coverage Δ
filter-true 70.54% <75.53%> (-1.40%) ⬇️
hana-1 ?
integration 70.54% <75.53%> (-1.40%) ⬇️
mongodb-1 5.21% <0.00%> (-0.01%) ⬇️
postgresql-1 50.01% <55.31%> (-0.05%) ⬇️
postgresql-2 51.11% <55.31%> (-0.03%) ⬇️
postgresql-3 51.81% <55.31%> (-0.05%) ⬇️
sqlite-1 49.15% <45.74%> (-0.05%) ⬇️
sqlite-2 50.21% <45.74%> (-0.06%) ⬇️
sqlite-3 50.99% <45.74%> (-0.01%) ⬇️
unit 30.97% <70.21%> (+0.22%) ⬆️

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

auto-merge was automatically disabled December 25, 2023 07:51

Head branch was pushed to by a user without write access

@sachinpuranik
Copy link
Contributor Author

fixed test , broken while fixing the linting.

@sachinpuranik sachinpuranik requested a review from henvic December 27, 2023 06:23
henvic
henvic previously approved these changes Dec 27, 2023
View reviewed changes
@AlekSi AlekSi enabled auto-merge (squash) December 27, 2023 13:59
AlekSi
AlekSi reviewed Dec 27, 2023
View reviewed changes
internal/backends/backend_test.go Outdated Show resolved Hide resolved
internal/backends/backend_test.go Outdated Show resolved Hide resolved
internal/backends/backend_test.go Outdated Show resolved Hide resolved
internal/backends/backend_test.go Outdated Show resolved Hide resolved
auto-merge was automatically disabled January 2, 2024 12:11

Head branch was pushed to by a user without write access

@sachinpuranik sachinpuranik requested review from AlekSi and henvic January 2, 2024 12:26
@sachinpuranik
Copy link
Contributor Author

sachinpuranik commented Jan 2, 2024 •
edited
Loading

Hi @AlekSi ,
PTAL Thanks.

@AlekSi AlekSi enabled auto-merge (squash) January 4, 2024 20:35
@AlekSi AlekSi merged commit 6a17ca1 into FerretDB:main Jan 4, 2024
28 of 29 checks passed
@AlekSi
Copy link
Member

AlekSi commented Jan 4, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henvic henvic henvic approved these changes

@AlekSi AlekSi AlekSi approved these changes

@rumyantseva rumyantseva Awaiting requested review from rumyantseva rumyantseva is a code owner automatically assigned from FerretDB/leads

@noisersup noisersup Awaiting requested review from noisersup

@ptrfarkas ptrfarkas Awaiting requested review from ptrfarkas

Assignees

@AlekSi AlekSi

@sachinpuranik sachinpuranik

Labels
code/chore Code maintenance improvements
Projects
Current Engineering
Archived in project
Milestone
v1.18.0
Development

Successfully merging this pull request may close these issues.

ListDatabases and ListCollections should allow filtering by name
5 participants
@sachinpuranik @CLAassistant @AlekSi @henvic @noisersup