-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add asset upload functionality #203
Conversation
19b24f4
to
631bd24
Compare
|
let mut conn = pool | ||
.acquire() | ||
.await | ||
.expect("Failed to get database connection."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we return a 500 instead of panicking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made an issue -- #215 -- cause this (failing gracefully) might take some time to figure out how to do properly
.layer(Extension(pool.clone())); | ||
|
||
let asset_route = Router::new() | ||
.route("/:namespace/:identifier", post(register_index_assets)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the idea here to have identifier
as the graph name and namespace
be something like an "org name"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll want a way of deconflicting name choices from different devs, something to inform them whether a name was taken, or help them choose a namespace name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Yea that's the idea.
namespace
is your org,identifier
is the name of that index running for that org - You're right we'll eventually want friendly messaging around what namespace's are available
- We would first need to implement some actual form of user authentication so namespaces can be tied to authenticated users
Description
anyhow
back to1.53
database
package into thefuel-indexer-database
crateIndexerConfig
logic fromfuel-indexer-lib/lib.rs
tofuel-indexer/config.rs
IndexerConnection
,IndexerConnectionPool
, and the queries) fromfuel-indexer-schema/{db, models}.rs
tofuel-indexer-database/{lib, queries}.rs
fuel-indexer-schema
intofuel-indexer-database
(e.g., the database model structs,ForiengKey
) but that can waitindex_asset_registry_*
tables which will hold the uploaded assets"identifier"
to an index -- with theidentifier
just being the name of the index$namesapce + $identifier
Some implementation details
IndexerService
👀cargo sqlx prepare --database-url sqlite:///path-to/my.db
🤷Testing steps
bash tests/e2e/composable-indexer/compose-up.bash
curl
command again -- you should still only see a single set of assets (duplicates aren't uploaded)examples/counter/manifest.yaml
then re-upload it using the below commandcounter.index1
index, and the latest version inindex_asset_registry_manifest
should be "2"fuel-indexer
Docker containerfuel-indexer
container again/ping
should still work (because the indices were loaded from the database when the service was started again)