Sam/json csv config

[sam-fs]

SaveAsJson is now a global config variable.

Changed example-config.toml default settings to CSV format, local storage, and GCS only so that a user has more and easier paths to a valid config.

Next step is adding checks and outputing appropriate error messages to the log so that the users can fix their config, but submitting this PR now to get quick fixes published while resolving some cloud credentialing issues.

[butanian]

I feel like the example config should propagate the data all the way to redshift/bq.

Is moving the json format option to the global scope a time sensitive issue? Was the motivation to support uploading json file to s3/gcs? Just wondering if we would be better off including config file validation in the same PR.

[butanian]

I think we can remove the trailing ? here and also in the above the S3Only property

[sam-fs]

The motivation was that the “default” config that existed didn't work. SaveAsJson=true was set in [local] in the .toml and that was incompatible with Warehouse="redshift". A user had to set SaveAsJson=true in [local] in order to get CSVs for loading into Redshift (or BigQuery), which wasn't immediately obvious to new users. Also, choosing JSON/CSV makes more sense as a global config b/c someone could want to upload JSON to S3 or GCS, not just download JSONs to local storage.

https://app.clubhouse.io/fullstory/story/62084/make-hauser-smarter-about-json-vs-csv

Looks like the CI checks failed due to GO1.11 config issue. Not sure what that's about.

[butanian]

To fix the problem, is it possible to only be sensitive to conf.Local.SaveAsJson if the selected warehouse is local?

My reasoning is that a global save as json flag implies, we create and upload json file, and then also support loading that format into bq and redshift, which we currently don't do. Thoughts?

[sam-fs]

BQ and Red won't accept JSONs directly, so I was thinking the next step would just be adding checks for those bad configs and using log.Fatalf to explain to the user how to correct them. This is how we handle the GroupFilesByDay global config.

…

On Wed, May 1, 2019 at 1:28 PM Aneesh Butani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In example-config.toml <#54 (comment)>: > @@ -33,7 +34,8 @@ DatabaseSchema = "public" [gcs] Bucket = "<your bucket>" -GCSOnly = false +# upload data file only (i.e. skip load to BigQuery)? To fix the problem, is it possible to only be sensitive to conf.Local.SaveAsJson if the selected warehouse is local? My reasoning is that a global save as json flag implies, we create and upload json file, and then also support loading that format into bq and redshift, which we currently don't do. Thoughts? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#54 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AK67X4G5UN225VP4TOGYWALPTHHKFANCNFSM4HJQKA4Q> .

-- Thanks, Sam

[butanian]

SGTM

the travis error was due to checkgofmt not being happy (probably spaces vs. tab on the line added in config.go). Did gofmt -s -w config/config.go to generate the commit I pushed.

[sam-fs]

Ah, noob oversight. :-| Thank you for pointing that out and making the correction.

…

On Wed, May 1, 2019 at 2:28 PM Aneesh Butani ***@***.***> wrote: SGTM the travis error was due to checkgofmt not being happy (probably spaces vs. tab on the line added in config.go). Did gofmt -s -w config/config.go to generate the commit I pushed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#54 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AK67X4D7DG7EGU43PELRGKDPTHOLZANCNFSM4HJQKA4Q> .

-- Thanks, Sam

[sam-fs]

This latest travis error may be due to main.go formatting. I've run gofmt -s -w on the file.

[jameremo]

LGTM, thanks for this work. We should look into setting up our IDEs or pre-commit hooks that run gofmt prior to commit, else we're all going to be super annoyed fixing builds that fail due to formatting. I'll follow up on that.

Also, cc @camphillips22, since he was doing some rearchitecting on hauser and will likely want to make sure this tweak gets incorporated in his branch.