Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

First Steps Towards Replacing sqlite_orm with sqlite_modern_cpp #238

Merged
merged 3 commits into from
Nov 13, 2023
Merged

First Steps Towards Replacing sqlite_orm with sqlite_modern_cpp #238

merged 3 commits into from
Nov 13, 2023

Conversation

irq0
Copy link
Member

@irq0 irq0 commented Oct 25, 2023 •
edited
Loading

Add sqlite_modern_cpp alongside sqlite_orm as a submodule. Wrap
include in dbapi.h to work around it's namespace sqlite coliding
with ours. Convert SQLiteList::versions and add type mappers for
ceph::real_time, ObjectState and VersionType

Epic: https://github.com/aquarist-labs/s3gw/issues/742
Issue: https://github.com/aquarist-labs/s3gw/issues/788

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)

@irq0 irq0 force-pushed the pr/sqlite-modern-cpp branch 5 times, most recently from d7a998f to 8d384d3 Compare October 25, 2023 16:54
@irq0 irq0 requested review from 0xavi0 and jecluis October 26, 2023 06:53
jecluis
jecluis reviewed Nov 2, 2023
View reviewed changes
Copy link
Member

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

lgtm - but lets only merge this after #233

@github-actions github-actions bot added the needs-rebase Changes need a rebase label Nov 6, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 6, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

0xavi0
0xavi0 previously approved these changes Nov 13, 2023
View reviewed changes
Copy link

@0xavi0 0xavi0 left a comment

Choose a reason for hiding this comment

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

lg2m.

I have ready the changes for all blobs in a generic way using this branch as the base, so I will push my PR after this is merged.
I have also the whole Users queries converted to the new sqlite library.

@jecluis
Copy link
Member

jecluis commented Nov 13, 2023

@irq0 we have a conflict. Lets address that so we can merge this asap, so we can avoid recurring conflicts.

rgw/sfs: Add sqlite_modern_cpp submodule
9a8c5fa 
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
@github-actions github-actions bot removed the needs-rebase Changes need a rebase label Nov 13, 2023
Marcel Lauhoff added 2 commits November 13, 2023 13:33
rgw/sfs: Wire sqlite_modern_cpp into sfs builds
b4bcbe5 
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
rgw/sfs: Convert SQLiteList::versions to sqlite_modern_cpp
7ee657f 
Convert versions listing to sqlite_modern_cpp and add all necessary
type bindings (ObjectState, VersionType, ceph::real_time)

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
@0xavi0 0xavi0 self-requested a review November 13, 2023 13:17
@irq0 irq0 merged commit 0e62a28 into aquarist-labs:s3gw Nov 13, 2023
6 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jecluis jecluis jecluis left review comments

@0xavi0 0xavi0 0xavi0 approved these changes

Assignees

@irq0 irq0

Labels
None yet
Projects
S3GW
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@irq0 @jecluis @0xavi0