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

Change config to yaml #51

Closed
zhilingc opened this issue Jan 8, 2019 · 4 comments
Closed

Change config to yaml #51

zhilingc opened this issue Jan 8, 2019 · 4 comments
Labels
kind/feature New feature or request

Comments

@zhilingc
Copy link
Collaborator

zhilingc commented Jan 8, 2019

Is your feature request related to a problem? Please describe.
Configuration for feast is currently done using environment variables only. As the number of parameters grows in size, this method of configuring the deployment becomes increasingly unwieldy.

Describe the solution you'd like
We want to move away from KVs, which are limited when it comes to nested config, eg. for job options. Currently we set it as a json string.

Describe alternatives you've considered
If we use yaml, it would potentially look something like this:

---
# CORE CONFIG #
grpc.port: 6565
http.port: 8080
feast.jobs:
  coreUri: localhost:6565
  runner: DataflowRunner
  options:
    project: my-dataflow-runner-project
    region: asia-east1
    tempLocation: gs://feast-temp-bucket #also required for bq ingestion
    subnetwork: regions/asia-east1/subnetworks/default
  executable: feast-ingestion.jar
  errorsStore:
    type: file.json
    options:
      path: gs://feast-errors
  monitoring:
    period: 5000
    initialDelay: 600000
    statsd:
      host: localhost
      port: 8125

# DB CONFIG #
spring.jpa.properties.hibernate.format_sql: true
spring.datasource.url: "jdbc:postgresql://localhost:5432/postgres"
spring.datasource.username: postgres
spring.datasource.password: password
spring.jpa.hibernate.naming.physical-strategy: org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
spring.jpa.hibernate.ddl-auto: update

# APP METRICS #
management.metrics.export.simple.enabled: false
management.metrics.export.statsd.enabled: true
management.metrics.export.statsd.host: localhost
management.metrics.export.statsd.port: 8125

We can still continue to support env vars for overriding config.

@zhilingc
Copy link
Collaborator Author

zhilingc commented Jan 8, 2019

@woop @tims thoughts?

@woop
Copy link
Member

woop commented Jan 8, 2019

I think it's worth doing. Easier to work with, and easier to read.

Why would we have some keys nested and some flattened? Why not all nested like:

spring:
  jpa:
    properties:
      format_sql: true
    hibernate
      naming
        physical-strategy: blah
      ddl-auto: update
  datasource:
    url:  "jdbc:postgresql://localhost:5432/postgres"
    username: postgres
    password: password

instead of

spring.jpa.properties.hibernate.format_sql: true
spring.datasource.url: "jdbc:postgresql://localhost:5432/postgres"
spring.datasource.username: postgres
spring.datasource.password: password
spring.jpa.hibernate.naming.physical-strategy: org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
spring.jpa.hibernate.ddl-auto: update

@zhilingc
Copy link
Collaborator Author

zhilingc commented Jan 8, 2019

yeah, that works too.

@tims
Copy link
Contributor

tims commented Jan 10, 2019

yaml sounds good to me

@pradithya pradithya added the kind/feature New feature or request label Jan 15, 2019
@woop woop closed this as completed Sep 1, 2019
dmartinol pushed a commit to dmartinol/feast that referenced this issue Jul 25, 2024
…missions

Fix auth tests with permissions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants