Add common interface to write exported files #215
Conversation
| func MustGcsFlags(flags *pflag.FlagSet, logger *EtlLogger) (bucket, credentials string) { | ||
| bucket, err := flags.GetString("gcs-bucket") | ||
| // MustCloudStorageFlags gets the values of the bucket list specific flags: cloud-storage-bucket, cloud-credentials | ||
| func MustCloudStorageFlags(flags *pflag.FlagSet, logger *EtlLogger) (bucket, credentials, provider string) { |
There was a problem hiding this comment.
By making this method required, if a user only wants to read/write the files locally and deal with transmission in another process, are they going to get a warning every time they export without specifying a storage destination?
There was a problem hiding this comment.
I almost wonder if we make writing to a data store something you opt into, like the --testnet parameter and then pass accompanying cloud provider flags
There was a problem hiding this comment.
are they going to get a warning every time they export without specifying a storage destination?
They will not
Well the funny thing is the precedent in the code is to make it required but give it a default value 😆
I figured we could refactor that along with the larger txmeta change or as other tickets. Where this refactor would include updating the cmd package (actually deprecating the unused export_* commands and making root.go more extensible), updating flags, and pulling out even more randomly embedded functions into separate util_* files
There was a problem hiding this comment.
Well the funny thing is the precedent in the code is to make it required but give it a default value 😆
yeah, yeah i know 😅
i like that idea, i'm fine incrementally improving this interface so we can clean it up once we refactor stellar-etl to use a different ledgerbackend interface (ie, reading from GCS)
cmd/export_orderbooks.go
Outdated
There was a problem hiding this comment.
At some point we should evaluate whether it makes sense to deprecate this entirely.
Note: Because of the way the cmd package was written the
command_utilsandupload_to_gcsare at the same cmd/ directory level in order to avoid more refactoring. This is mostly due to cmdLogger (from root.go) and being embedded in a bunch of functions throughout the cmd package