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

Bump sqlx to 0.7.4 #4959

Merged
merged 3 commits into from
Oct 18, 2024
Merged

Bump sqlx to 0.7.4 #4959

merged 3 commits into from
Oct 18, 2024

Conversation

dynco-nym
Copy link
Contributor

@dynco-nym dynco-nym commented Oct 10, 2024

For sanity, please run all of these

cd nym-data-observatory && cargo check
cd common/credential-storage && cargo check
cd common/gateway-storage && cargo check
cd common/client-core/gateways-storage && cargo check --features fs-gateways-storage

This change is Reviewable

Copy link

vercel bot commented Oct 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
nym-explorer ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2024 10:09am
nym-next-explorer ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2024 10:09am

@dynco-nym dynco-nym self-assigned this Oct 10, 2024
@dynco-nym dynco-nym added this to the Magura milestone Oct 10, 2024
@dynco-nym dynco-nym force-pushed the andrej/sqlx-0.7 branch 2 times, most recently from 0717e69 to 367706b Compare October 11, 2024 08:30
Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 166 files at r1.
Reviewable status: 6 of 166 files reviewed, 1 unresolved discussion (waiting on @dynco-nym and @octol)


common/client-core/gateways-storage/.sqlx/query-0f1dfb89f1eb39f4a58787af0f53a7a93afb7e4d2e54e2d38fd79d31c8575a54.json line 3 at r1 (raw file):

{
  "db_name": "SQLite",
  "query": "\n                UPDATE remote_gateway_details\n                SET\n                    derived_aes128_ctr_blake3_hmac_keys_bs58 = ?,\n                    derived_aes256_gcm_siv_key = ?\n                WHERE gateway_id_bs58 = ?\n            ",

This is an odd looking query. Is it supposed to be that way?

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewed 160 of 166 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @octol)


nym-api/src/ecash/storage/manager.rs line 371 at r1 (raw file):

            IssuedTicketbook,
            r#"
                SELECT 

FYI I hope clearing this whitespace does not break the query

Copy link
Contributor

@octol octol left a comment

Choose a reason for hiding this comment

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

Looks good to me, but also no sqlite expert ...

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dynco-nym)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dynco-nym and @jstuczyn)


nym-api/src/support/storage/manager.rs line 556 at r1 (raw file):

Previously, dynco-nym (Dinko Zdravac) wrote…

The Executor impls for Transaction and PoolConnection have been deleted because they cannot exist in the new crate architecture without rewriting the Executor trait entirely.
To fix this breakage, simply add a dereference where an impl Executor is expected, as they both dereference to the inner connection type which will still implement it:

mentioned here

I think .as_mut() is seemingly the same as &mut *tx

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dynco-nym and @jstuczyn)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dynco-nym and @jstuczyn)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Tested on macOS and Windows (ARM64). All cargo checks passed.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dynco-nym and @jstuczyn)

Copy link
Contributor

@jstuczyn jstuczyn left a comment

Choose a reason for hiding this comment

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

Reviewed 166 of 166 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dynco-nym)

pronebird and others added 3 commits October 18, 2024 12:06
More goodies

Revert cargo.toml

Fix cargo toml
Fix data-observatory

Fix credential-storage

Fix gateway-storage

Fix client-core/-gateways-storage

Generate offline schemas for nym-api as well
@dynco-nym dynco-nym merged commit 4e65617 into develop Oct 18, 2024
23 of 24 checks passed
@dynco-nym dynco-nym deleted the andrej/sqlx-0.7 branch October 18, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants