-
Notifications
You must be signed in to change notification settings - Fork 175
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
feat(store-sync): add util to fetch snapshot from dozer #2996
base: main
Are you sure you want to change the base?
Conversation
|
0b6142c
to
f22747a
Compare
schemaAbiTypeToDefaultValue, | ||
} from "@latticexyz/schema-type/internal"; | ||
|
||
export function decodeDozerField<abiType extends AbiType>( |
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.
what about decodeField
or decodeColumnValue
? (we're in the dozer export, so dozer in the name seems duplicative)
records: DecodeDozerRecordsResult; | ||
}[]; | ||
} | ||
| undefined; |
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.
not blocking but fwiw I found it quite nice to model this sort of thing with arktype, then can parse/validate/return strongly typed response from a JSON API request
const response: DozerResponse = await ( | ||
await fetch(dozerUrl, { | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
}, | ||
body: JSON.stringify(queries.map((query) => ({ address: storeAddress, query: query.sql }))), | ||
}) | ||
).json(); | ||
|
||
if (isDozerResponseFail(response)) { | ||
console.warn(`Dozer response: ${response.msg}\n\nTry reproducing via cURL: | ||
curl ${dozerUrl} \\ | ||
--compressed \\ | ||
-H 'Accept-Encoding: gzip' \\ | ||
-H 'Content-Type: application/json' \\ | ||
-d '[${queries.map((query) => `{"address": "${storeAddress}", "query": "${query.sql.replaceAll('"', '\\"')}"}`).join(",")}]'`); | ||
return; | ||
} |
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 find it a bit easier to read
await fetch(...).then((res) => res.json())
than nested awaits - lifted stringified
json
up so we can reuse it in curl command output
const response: DozerResponse = await ( | |
await fetch(dozerUrl, { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
}, | |
body: JSON.stringify(queries.map((query) => ({ address: storeAddress, query: query.sql }))), | |
}) | |
).json(); | |
if (isDozerResponseFail(response)) { | |
console.warn(`Dozer response: ${response.msg}\n\nTry reproducing via cURL: | |
curl ${dozerUrl} \\ | |
--compressed \\ | |
-H 'Accept-Encoding: gzip' \\ | |
-H 'Content-Type: application/json' \\ | |
-d '[${queries.map((query) => `{"address": "${storeAddress}", "query": "${query.sql.replaceAll('"', '\\"')}"}`).join(",")}]'`); | |
return; | |
} | |
const json = JSON.stringify(queries.map((query) => ({ address: storeAddress, query: query.sql }))); | |
const response: DozerResponse = fetch(dozerUrl, { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
}, | |
body: json, | |
}).then((res) => res.json(); | |
if (isDozerResponseFail(response)) { | |
console.warn(`Dozer response: ${response.msg}\n\nTry reproducing via cURL: | |
curl ${dozerUrl} \\ | |
--compressed \\ | |
-H 'Accept-Encoding: gzip' \\ | |
-H 'Content-Type: application/json' \\ | |
-d '${json.replaceAll("'", "\\'")}'`); | |
return; | |
} |
} | ||
| undefined; | ||
|
||
export async function fetchRecordsSql({ |
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.
what about just fetchRecords
? or query
? it's dozer so I think SQL can be implied
storeAddress, | ||
}: FetchRecordsSqlArgs): Promise<FetchRecordsSqlResult> { | ||
const response: DozerResponse = await ( | ||
await fetch(dozerUrl, { |
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.
since this URL we're fetching is tied to the shape of the request/response, I wonder if this arg should be the endpoint/hostname and we append the path like /q
?
-H 'Accept-Encoding: gzip' \\ | ||
-H 'Content-Type: application/json' \\ | ||
-d '[${queries.map((query) => `{"address": "${storeAddress}", "query": "${query.sql.replaceAll('"', '\\"')}"}`).join(",")}]'`); | ||
return; |
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 find it a little weird that a function to fetch a thing returns undefined on a failure rather than throwing or something.
I would usually reserve this behavior for the place where "failure is optional" e.g. getSnapshot
or syncToStash
no changes expected, just curious to hear your reasoning
blockNumber: results.length > 0 ? bigIntMin(...results.map((result) => result.blockNumber)) : startBlock, | ||
logs: results.flatMap((result) => result.logs), | ||
}; | ||
|
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.
ooc what happens if fetchLogs
fails but fetchSql
doesn't? or vice versa? or some of fetchSql
does? do we end up in a weird state?
|
||
const fetchSql = (): Promise<StorageAdapterBlock | undefined>[] => { | ||
return sqlFilters.map(async (filter) => { | ||
const result = await fetchRecordsSql({ dozerUrl, storeAddress, queries: [filter] }); |
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.
more of a stylistic thing but I wonder if this would be more readable with
- function for each query that we can
.map
with later - early return when no results
const fetchRecords = async (query: TableQuery): Promise<StorageAdapterBlock | undefined> => {
const result = await fetchRecordsSql({ dozerUrl, storeAddress, queries: [filter] });
if (!result) return;
return {
const results = (await Promise.all([fetchLogs(), ...sqlFilters.map(fetchRecords)])).filter(isDefined);
import { TableQuery } from "./common"; | ||
|
||
// For autocompletion but still allowing all SQL strings | ||
export type Where<table extends Table> = `"${keyof table["schema"] & string}"` | (string & {}); |
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.
arktype has autocomplete<...>
util for this!
https://github.com/arktypeio/arktype/blob/16f254a57715d2b446c94a269cabb32bd36cfec3/ark/util/generics.ts#L80-L82
export type GetSnapshotArgs = { | ||
dozerUrl: string; | ||
storeAddress: Hex; | ||
filters?: SyncFilter[]; |
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.
would it better or worse to split this into two args: filters
and queries
?
No description provided.