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

Config file support, env var support, application and streamlet level support. #113

Closed
wants to merge 14 commits into from

Conversation

@RayRoestenburg
Copy link
Contributor

RayRoestenburg commented Jan 21, 2020

What changes were proposed in this pull request?

  • Support passing through config files, which are automatically merged in order specified (last takes most precedence)
  • Support application level configuration (pass through config for all streamlets)
  • Support streamlet specific configuration (using the application-level section, it's content is copied to the root of the conf)
  • Support env var replacements (env vars in the env where kubectl cloudflow is running)
  • Allow config args to pass through subsystem config

Why are the changes needed?

Improvements to configuration system. This prepares for other planned changes, to be able eto easily configure any aspect of a cloudflow application.

Does this PR introduce any user-facing change?

Yes, users will be able to pass config files in HOCON format to kubectl-cloudflow. configuration parameters used in the file are validated, just like the arguments are validated. It is possible to pass through config that should not be validated with configuration parameters.

How was this patch tested?

Tested on a Cloudflow cluster, as wel as unit testing.

@RayRoestenburg RayRoestenburg changed the title WIP: Support `--conf config-file1 config-file2 ...` WIP: Support `--conf config-file1 --conf config-file2 ...` Jan 21, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch 8 times, most recently from ed8ec99 to d1cc2b5 Jan 23, 2020
@RayRoestenburg RayRoestenburg marked this pull request as ready for review Jan 23, 2020
@RayRoestenburg RayRoestenburg changed the title WIP: Support `--conf config-file1 --conf config-file2 ...` Config file support, env var support, application and streamlet level support. Jan 23, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from d1cc2b5 to 4ab5e4f Jan 24, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from d68af85 to 1d984bf Jan 24, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from 1d984bf to 4c49ee8 Jan 25, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from 8f09337 to a2bd7ba Jan 25, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from a2bd7ba to e004c48 Jan 25, 2020
@RayRoestenburg RayRoestenburg force-pushed the ray/config-file-support branch from 8f0219c to 1e0ea70 Jan 26, 2020
@@ -33,6 +34,7 @@ require (
github.com/opencontainers/go-digest v1.0.0-rc1 // indirect
github.com/opencontainers/image-spec v1.0.1 // indirect
github.com/pkg/errors v0.8.1 // indirect
github.com/rayroestenburg/configuration v0.0.0-20200115015912-550403a6bd87

This comment has been minimized.

Copy link
@skonto

skonto Feb 10, 2020

Contributor

Do we allow our own repo deps here? Shouldnt we only depend on github.com/go-akka/configuration and not our fork?

This comment has been minimized.

Copy link
@RayRoestenburg

RayRoestenburg Feb 10, 2020

Author Contributor

I wanted to make sure we can add fixes directly.

@@ -151,6 +153,8 @@ github.com/prometheus/common v0.2.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y8
github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/rayroestenburg/configuration v0.0.0-20200115015912-550403a6bd87 h1:JTGDqHa23LTUjEsJQorToCYedKxI6SM9SkhooYyKlTw=

This comment has been minimized.

Copy link
@skonto

skonto Feb 10, 2020

Contributor

pls rm your repo references.

This comment has been minimized.

Copy link
@RayRoestenburg

RayRoestenburg Feb 10, 2020

Author Contributor

This is auto-generated

This comment has been minimized.

Copy link
@skonto

skonto Feb 10, 2020

Contributor

I dont have it on my setup and I can use that lib as is, but I understand it is used so we can test it and apply fixes.

@agemooij

This comment has been minimized.

Copy link
Contributor

agemooij commented Feb 21, 2020

Discussed offline. Due to some design discussions we have updated the spec for the new config system and that makes this Pr outdated. We will submit a new one based on the latest spec.

@agemooij agemooij closed this Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.