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

yaml configuration is error-prone #1859

Open
Ubehebe opened this issue Apr 20, 2021 · 1 comment
Open

yaml configuration is error-prone #1859

Ubehebe opened this issue Apr 20, 2021 · 1 comment
Labels
config webserver Jetty & web actions

Comments

@Ubehebe
Copy link
Contributor

Ubehebe commented Apr 20, 2021

I maintain a Misk-based app. Yesterday, I added a backend to it via Guice injection, and added fake and real Guice modules for the binding. All tests passed. But when I deployed the change to the app's staging environment, it failed:

com.google.inject.ProvisionException: Unable to provision, see the following errors:
        1) Error in custom provider, java.lang.IllegalArgumentException: no client configuration for endpoint [redacted]
          at misk.client.HttpClientModule.configure(HttpClientModule.kt:23) (via modules: [redacted] -> misk.client.TypedHttpClientModule -> misk.client.HttpClientModule)
          while locating okhttp3.OkHttpClient annotated with @com.google.inject.name.Named(value=[redacted])
          at misk.client.TypedHttpClientModule.configure(TypedHttpClientModule.kt:45) (via modules: [redacted] -> misk.client.TypedHttpClientModule)
          while locating [redacted] annotated with @com.google.inject.name.Named(value=[redacted])

After reading Misk docs, I realized I needed to add a line to app-staging.yaml corresponding to this new backend.

Could this error have been surfaced before deploying? I realize that app-staging.yaml is meant for the staging environment, but it should be possible to write a tool that would do some static analysis on the yaml file and realize there's a missing backend. (I'm filing this issue here, rather than against my own app, because it seems like it would be valuable for every Misk app.)

@r3mariano
Copy link
Collaborator

Sorry about the long delay in replying.

I think that a test on the Injector with different envs should have picked this up.

While we've added config-checking via hoplite, that particular error stems from the module trying to pull dynamically from the config, so without changing how the module works, an injector test is probably the way to go.

@r3mariano r3mariano added config webserver Jetty & web actions labels Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config webserver Jetty & web actions
Projects
None yet
Development

No branches or pull requests

2 participants