-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update to use BufferedStorageBackend to read txmeta files #242
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: golang/github.com/stellar/[email protected] |
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.
Nothing blocking the merge. Changes look good
Might be worthwhile for us to discuss the operator experience for stellar-etl since we're also discussing for Ledger Exporter. Having the flags called through multiple methods feels weird now that we're deleting our extract from genesis code. Specifically, it might be worthwhile for us to collapse MustCommonFlags
and MustArchiveFlags
cmd/export_account_signers.go
Outdated
env := utils.GetEnvironmentDetails(isTest, isFuture, datastoreUrl) | ||
commonArgs := utils.MustCommonFlags(cmd.Flags(), cmdLogger) | ||
cmdLogger.StrictExport = commonArgs.StrictExport | ||
env := utils.GetEnvironmentDetails(commonArgs.IsTest, commonArgs.IsFuture, commonArgs.DatastorePath) |
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 for this PR: wdty about passing an env arg instead of IsTest
and IsFuture
? I think we'd need to plan the breaking change with Goldsky, but might be a better UX
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.
Yeah that'd be good. We need to refactor a lot of the params TBH
Hubble-386 although it doesn't have a description lol
cmd/export_ledger_transaction.go
Outdated
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.
Todo: This export process may be less useful now that we save raw XDR to GCS
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.
Hmm that's true. Throwing it in the ticket lol
Hubble-386
} | ||
|
||
// TODO: In the future CompressionType should be removed as it won't be configurable | ||
BSBackendConfig := ledgerbackend.BufferedStorageBackendConfig{ |
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.
lol, BSBackendConfig :eyeroll:
Replaces
GCSBackend
with the releasedBufferedStorageBackend
ledgerbackend reader in the go monorepoBufferedStorageBackend
datastore-url
is nowdatastore-path
as the backend only takes the pathNetwork
added to env vars