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 building without PostgreSQL or SQLite backend #3803

Merged
merged 8 commits into from
Dec 4, 2023
Merged

Allow building without PostgreSQL or SQLite backend #3803

merged 8 commits into from
Dec 4, 2023

Conversation

anunayasri
Copy link
Contributor

@anunayasri anunayasri commented Dec 2, 2023 •
edited by AlekSi
Loading

Description

Requirement: We should allow building FerretDB without PostgreSQL and/or SQLite backend with ferretdb_no_postgresql and ferretdb_no_sqlite build tags. Both should be absent in official builds.

I have added the relevant build tags in postgres & sqlite handler and registry files. The corresponding test TestDeps is skipped in the main branch. I have added placeholders for testing the above build tag changes. We can fix these once the TestDeps is fixed.

Testing Strategy

  1. Add required build tags in BUILD_TAGS in Taskfile.yml.
  2. Build using task build-host.
  3. Check the compiled handlers using bin/ferretdb -h.

Test Results

  1. Build without Postgresql

Build tags given
image

Postgres handler is absent.
image

  1. Build without Sqlite

Build tags given
image

Sqlite handler is absent.
image

Closes #3629

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.

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@AlekSi
Copy link
Member

AlekSi commented Dec 2, 2023

@anunayasri please click the CLA button in the comment above

@anunayasri anunayasri marked this pull request as ready for review December 2, 2023 07:34
@anunayasri anunayasri requested review from AlekSi and a team as code owners December 2, 2023 07:34
@codecov Codecov
Copy link

codecov bot commented Dec 2, 2023 •
edited
Loading

Codecov Report

Merging #3803 (f2a2104) into main (126b59d) will decrease coverage by 5.98%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3803      +/-   ##
==========================================
- Coverage   76.35%   70.37%   -5.98%     
==========================================
  Files         315      315              
  Lines       23540    23540              
==========================================
- Hits        17973    16566    -1407     
- Misses       4528     5872    +1344     
- Partials     1039     1102      +63
Files Coverage Δ
build/version/version.go 67.64% <ø> (ø)
internal/handler/registry/postgresql.go 77.77% <ø> (ø)
internal/handler/registry/sqlite.go 77.77% <ø> (ø)

... and 61 files with indirect coverage changes

Flag Coverage Δ
filter-true 65.98% <ø> (-6.52%) ⬇️
hana-1 ?
integration 65.98% <ø> (-6.52%) ⬇️
mongodb-1 5.19% <ø> (ø)
postgresql-1 52.33% <ø> (-0.03%) ⬇️
postgresql-2 51.28% <ø> (ø)
postgresql-3 ?
sqlite-1 51.38% <ø> (-0.04%) ⬇️
sqlite-2 50.43% <ø> (ø)
sqlite-3 ?
unit 30.73% <ø> (ø)

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

AlekSi
AlekSi reviewed Dec 2, 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.

Well, build tags look good, but we need to update the test

build/version/version.go Outdated Show resolved Hide resolved
build/version/version.go Outdated Show resolved Hide resolved
@AlekSi
Copy link
Member

AlekSi commented Dec 2, 2023

The corresponding test TestDeps is skipped in the main branch

Why do you think so?

@anunayasri
Copy link
Contributor Author

The corresponding test TestDeps is skipped in the main branch

Why do you think so?

@AlekSi I see my mistake. The tests were fixed recently but I didn't take the pull. I have fixed the tests. Please have a look.

anunayasri and others added 5 commits December 4, 2023 08:49
@anunayasri
Build with postgresql or sqlite using build tags
7b4c830
@anunayasri @AlekSi
Fix typo in build/version/version.go
98aaa69 
Co-authored-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com>
@anunayasri @AlekSi
Fix typo in build/version/version.go
f921750 
Co-authored-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com>
@anunayasri
Add tests for Neg build tags
982c436
@anunayasri
Fix ci failure
366a642
@anunayasri anunayasri force-pushed the build-without-postgres-sqlite branch from 35d386a to 366a642 Compare December 4, 2023 03:19
@AlekSi AlekSi changed the title Allow building ferretdb without PostgreSQL or SQLite backend Allow building FerretDB without PostgreSQL or SQLite backend Dec 4, 2023
@AlekSi
Copy link
Member

AlekSi commented Dec 4, 2023

FerretDB/CONTRIBUTING.md

Lines 362 to 364 in ced8ce0

All commits are always squashed on merge by GitHub.
Please **do not** squash them manually, amend them, and/or force push them -
that makes the review process harder.

@AlekSi AlekSi added this to the Next milestone Dec 4, 2023
@AlekSi AlekSi added the code/feature Some user-visible feature is not implemented yet label Dec 4, 2023
@anunayasri anunayasri requested a review from AlekSi December 4, 2023 13:28
@AlekSi AlekSi requested review from a team, henvic and noisersup December 4, 2023 16:21
@AlekSi AlekSi enabled auto-merge (squash) December 4, 2023 16:22
@AlekSi AlekSi changed the title Allow building FerretDB without PostgreSQL or SQLite backend Allow building without PostgreSQL or SQLite backend Dec 4, 2023
@AlekSi AlekSi disabled auto-merge December 4, 2023 16:23
@AlekSi AlekSi enabled auto-merge (squash) December 4, 2023 16:23
@AlekSi AlekSi merged commit 7aefd7d into FerretDB:main Dec 4, 2023
26 of 27 checks passed
@anunayasri anunayasri deleted the build-without-postgres-sqlite branch December 5, 2023 03:11
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 noisersup was automatically assigned from FerretDB/core

Assignees

@anunayasri anunayasri

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

Successfully merging this pull request may close these issues.

Allow building without PostgreSQL or SQLite backend
4 participants
@anunayasri @CLAassistant @AlekSi @henvic