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

Contextualize database and separate in to modules #486

Merged
merged 8 commits into from
Jan 30, 2025

Conversation

ceyhunsen
Copy link
Member

@ceyhunsen ceyhunsen commented Jan 28, 2025

Description

Separates database/common into different files based on the context.

This PR basically moves code to separate modules but it also removes tracing::instruments (because they are useless) and adds some todos regarding to tests etc..

Adds execute_query_with_tx macro that will simplify query execution.

Linked Issues

@ceyhunsen ceyhunsen changed the base branch from main to dev January 28, 2025 14:36
@ceyhunsen ceyhunsen force-pushed the ceyhun/contextualize_database branch from fbf3bc3 to cec608b Compare January 29, 2025 07:53
@mmtftr
Copy link

mmtftr commented Jan 29, 2025

If you have the time, consider using this (I had written this earlier):

macro_rules! execute_with_tx {
    ($conn:expr, $tx:expr, $query:expr, $method:ident) => {
        match $tx {
            Some(tx) => $query.$method(&mut **tx).await,
            None => $query.$method(&$conn).await,
        }
    };
}


// later
execute_with_tx!(self.connection, tx, query, execute); 

@ceyhunsen
Copy link
Member Author

If you have the time, consider using this (I had written this earlier):

macro_rules! execute_with_tx {
    ($conn:expr, $tx:expr, $query:expr, $method:ident) => {
        match $tx {
            Some(tx) => $query.$method(&mut **tx).await,
            None => $query.$method(&$conn).await,
        }
    };
}


// later
execute_with_tx!(self.connection, tx, query, execute); 

Let me do this in a chain PR and not congest this PR, as there is already too many changes.

@ceyhunsen ceyhunsen added the HOLD MERGE Hold the merge while you see this label label Jan 29, 2025
@ceyhunsen ceyhunsen self-assigned this Jan 29, 2025
@ceyhunsen ceyhunsen marked this pull request as ready for review January 29, 2025 08:39
@ceyhunsen ceyhunsen added the code quality Improves code quality while not affecting any other label Jan 29, 2025
* database: Add execute_query_with_tx macro.

* database: Use execute_query_with_tx.

* Update core/src/database/mod.rs

Co-authored-by: Mehmet Efe Akça <[email protected]>

---------

Co-authored-by: Mehmet Efe Akça <[email protected]>
@ceyhunsen ceyhunsen removed the HOLD MERGE Hold the merge while you see this label label Jan 29, 2025
Copy link

@mmtftr mmtftr left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the changes, I think we'll improve in terms of documentation with these small changes

core/src/database/verifier.rs Outdated Show resolved Hide resolved
core/src/database/watchtower.rs Outdated Show resolved Hide resolved
core/src/database/verifier.rs Outdated Show resolved Hide resolved
core/src/database/watchtower.rs Outdated Show resolved Hide resolved
@ceyhunsen
Copy link
Member Author

Let me make this changes in a chain PR again for an easier review experience. We should keep this PR clean from refactors/updates. I made a mistake merging the child PR #488.

After every chain PR is approved, we can merge them one by one. If we don't do this, some changes will be missed by the reviewers.

@ceyhunsen ceyhunsen added the HOLD MERGE Hold the merge while you see this label label Jan 29, 2025
* database: Add the new DatabaseTransaction type.

* database: Update comments and names of watchtower functions.

* database: Update verifier func comments/names.

* database: Update comments and add tests for operator.

* database: Remove Option from DatabaseTransaction type.

* database: Update header chain prover namings and docs.
@ceyhunsen ceyhunsen removed the HOLD MERGE Hold the merge while you see this label label Jan 30, 2025
@ceyhunsen ceyhunsen merged commit f4219c3 into dev Jan 30, 2025
9 checks passed
@ceyhunsen ceyhunsen deleted the ceyhun/contextualize_database branch January 30, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improves code quality while not affecting any other
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split database/common into submodules
3 participants