Skip to content
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

Refactors config to separate general config from flow config #6

Merged
merged 8 commits into from
Nov 20, 2020

Conversation

jlerche
Copy link
Contributor

@jlerche jlerche commented Nov 19, 2020

This PR gets the ball rolling for eventually implementing vhsd. When running vhs, it's essentially a single run, but with vhsd we can expect it to run indefinitely while running many flows. Starting up vhsd should not require any flow specific configuration, but will probably share a lot of config settings with a vhs run.

@jlerche jlerche self-assigned this Nov 19, 2020
Copy link
Contributor

@ztreinhart ztreinhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. 👍

.gitignore Show resolved Hide resolved
Copy link
Contributor

@andrewhare andrewhare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! A couple changes but looks good.

session/config.go Outdated Show resolved Hide resolved
session/config.go Show resolved Hide resolved
Copy link
Contributor

@andrewhare andrewhare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good, but I need one more change and it's my bad for not noticing this on the first round.

These config settings also need to be on FlowConfig:

vhs/session/config.go

Lines 7 to 11 in ef2581e

Addr string
CaptureResponse bool
Middleware string
TCPTimeout time.Duration
HTTPTimeout time.Duration

vhs/session/config.go

Lines 15 to 18 in ef2581e

GCSBucketName string
GCSObjectName string
BufferOutput bool

These config settings get used by the flow and can change from run to run.

Copy link
Contributor

@andrewhare andrewhare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@jlerche jlerche merged commit 4e6c660 into main Nov 20, 2020
@jlerche jlerche deleted the break_out_flow_struct_from_config branch November 20, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants