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

Store offline data in one file per query / remove sqlx-data.json #941

Closed
wants to merge 3 commits into from

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Dec 29, 2020

Query data is now stored in .sqlx/{query_hash}.json directly by the macro invocations, rather than first writing to target/sqlx/{input_span_hash}.json and then collecting those into sqlx-data.json separately.

  • save offline data (if .sqlx exists)
  • load offline data
  • create .sqlx when running cargo sqlx prepare
  • diff target/sqlx with .sqlx in cargo sqlx prepare --check
  • update documentation

Closes #570.

@jplatte jplatte changed the title Rip sqlx prepare Store offline data in one file per query / remove sqlx-data.json and sqlx prepare command Dec 29, 2020
@jplatte jplatte force-pushed the rip-sqlx-prepare branch 2 times, most recently from 4529492 to 8e15d56 Compare December 30, 2020 13:08
@jplatte jplatte changed the title Store offline data in one file per query / remove sqlx-data.json and sqlx prepare command Store offline data in one file per query / remove sqlx-data.json Dec 30, 2020
@jplatte jplatte force-pushed the rip-sqlx-prepare branch 2 times, most recently from 2e28295 to f1d7770 Compare December 30, 2020 14:29
@jplatte
Copy link
Contributor Author

jplatte commented Dec 30, 2020

@mehcode @abonader it would be nice if one of you could do an initial review now, to find any issues with the general approach I took early. There are TODO comments for everything that's not done yet.

The only thing I'm not sure about is how support for workspaces with multiple crates using SQLx'es offline feature should work. I think it makes sense for there not to be one merged .sqlx directory as there was one merged sqlx-data.json before. We could probably detect whether there are multiple crates using sqlx with features = ["offline"] in the workspace and trigger the more complicated clean (maybe just of those crates) + check with RUSTFLAGS logic based on that. By the way, why does the cargo rustc code in there pass --emit dep-info,metadata but the cargo check code doesn't?

sqlx-macros/src/query/mod.rs Outdated Show resolved Hide resolved
by doing all .env and environment variable reading inside a Lazy initializer.
@jplatte jplatte force-pushed the rip-sqlx-prepare branch 2 times, most recently from ba643dc to 9a02f56 Compare February 2, 2021 14:55
Query data is now stored in .sqlx/{query_hash}.json directly by the macro
invocations, rather than first writing to target/sqlx/{input_span_hash}.json
and then collecting those into sqlx-data.json separately.
@mehcode
Copy link
Member

mehcode commented Feb 27, 2021

@jplatte What's left here? Is it just the bikeshedding over where we output the query data files?

@jplatte
Copy link
Contributor Author

jplatte commented Feb 27, 2021

No, I still need to get back to this and finish the implementation. I think all open questions were resolved. See the PR description for what's left to do.

@abonander
Copy link
Collaborator

abonander commented Mar 3, 2021

I think you'd still need a separate program to go through and remove stale query data to keep the folder size from growing indefinitely. Perhaps we could do something with build scripts where we put all the boilerplate in a function that the user can call in a single line, e.g.

fn main() {
    sqlx::build_script().unwrap();
}

which would:

  • emit cargo:rerun-if-changed:... for the files in .sqlx/ as well as all query_file!() paths (which would fix that invalidation problem) and migration files as well: Force recompile when migrations changed #681
  • if SQLX_OFFLINE is not set but DATABASE_URL is, wipe .sqlx/ to remove stale data

@jplatte
Copy link
Contributor Author

jplatte commented Mar 3, 2021

I know the branch name says rip-sqlx-prepare but I'm not actually planning to remove it anymore. I think the build script might be problematic if multiple compilations run at once, so I won't work on that in this PR. Also I think recompiling when a query_file! path changes shouldn't require a build script I think.

@jplatte
Copy link
Contributor Author

jplatte commented Apr 20, 2021

Huh, so even if a branch rename happens through GitHubs web UI, corresponding PRs are just closed. That's stupid...

@jplatte jplatte restored the rip-sqlx-prepare branch April 20, 2021 09:07
@jplatte jplatte deleted the rip-sqlx-prepare branch April 20, 2021 09:08
@jplatte
Copy link
Contributor Author

jplatte commented Apr 20, 2021

I wanted to re-open after renaming the branch back, but apparently that's not possible either. New PR is up: #1183.

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.

Move to one-file-per-query for sqlx-cli prepare
3 participants