cloudfoundry / notifications

The notifications service component of Cloud Foundry
Apache License 2.0
17 stars 22 forks source link

No guard for DATABASE_URL env var #19

Open osis opened 5 years ago

osis commented 5 years ago

While setting up a new environment, I noticed I started getting this error...

Test Panicked
  Error 1251: Client does not support authentication protocol requested by server; consider upgrading MariaDB client
  /usr/local/go/src/runtime/panic.go:491

Which was a bit alarming, but I think it's actually the result of not setting this environment variable.

Tracked it down to maybe this part of the source not functioning properly?

Also noticed that the tests don't cover this case. https://github.com/cloudfoundry-incubator/notifications/blob/master/application/environment_test.go#L63

cf-gitbot commented 5 years ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

osis commented 5 years ago

Turns out it was that was surfacing this configuration issue.

https://github.com/mysqljs/mysql/issues/1507#issuecomment-242885003

The error that you eventually cascades down to...

Test Panicked
  Error 1049: Unknown database 'notifications_test'

still, a "DOCKER_URL not set"-like error would be a nicer experience.