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

Basic travis config with postgres #13

Merged
merged 10 commits into from Nov 15, 2018
Merged

Conversation

bryanhuntesl
Copy link
Contributor

No description provided.

@sescobb27
Copy link
Contributor

@bryanhuntesl tests are not passing

The command "PGPASSWORD=postgres until psql -h localhost -U "postgres" -P $RELEASE_ADMIN_POSTGRES_PORT -c '\q' &> /dev/null ; do printf '.'; sleep 1; done" failed and exited with 1 during .

@sescobb27
Copy link
Contributor

what about just using the Postgres service from travis ci? if at some point we need to upgrade or need a custom feature or something we can spend more time on this. what do you think?

@bryanhuntesl
Copy link
Contributor Author

Compiling 42 files (.ex)
warning: variable "github_oauth" is unused
test/support/stubs_generation.ex:5
Generated release_admin app
** (Postgrex.Error) ERROR 42704 (undefined_object): type "jsonb" does not exist

@bryanhuntesl
Copy link
Contributor Author

bryanhuntesl commented Nov 9, 2018

@sescobb27 - postgres provided by travis does not support jsonb (didn't realise we were using it).

My code exposes env var RELEASE_ADMIN_POSTGRES_PORT at build time (corresponding to whatever port our docker postgres container runs on, but our config is hard coded to use localhost on default port :

https://github.com/sescobb27/release_admin/blob/107fdba2340862a6822d9750e72450f51db93676/config/test.exs#L16-L22

How would you like to proceed? I'm at code mesh today so availability will be patchy.

@sescobb27
Copy link
Contributor

hmmm as the Postgres docker image says, the default port is 5432 we don't need all that stuff because we already know the port, and as that port is the default we don't need to set up RELEASE_ADMIN_POSTGRES_PORT mostly because it will become a pain in dev/test in our local env. i think we can use de defaults for dev/test so we don't over complicate things, for now.

@bryanhuntesl
Copy link
Contributor Author

When it generates the message :

Compiling 42 files (.ex)
warning: variable "github_oauth" is unused
test/support/stubs_generation.ex:5
Generated release_admin app
** (Postgrex.Error) ERROR 42704 (undefined_object): type "jsonb" does not exist

That's release_admin talking to the travis postgres instance on the default port - the docker instance is running but release_admin is not talking to it.

Would be nice if the configuration could be communcated to this application via env vars ala The twelve factor app (config).

Does that make sense?

@bryanhuntesl
Copy link
Contributor Author

Hmmm. Maybe we should also consider if we can reduce the number of config files as a secondary objective.

screenshot 2018-11-09 at 13 42 11

@sescobb27
Copy link
Contributor

Ok got it, then, can use ReleaseAdmin.Repo.init function for getting config from runtime using System.get_env or RuntimeConfig module with postgres default values as defaults if no ENV was set, thoughts?

@bryanhuntesl
Copy link
Contributor Author

Yes something like that - would be nice if we had a single config module that handled all this stuff in one place rather than having it (reading env vars) spread all over the codebase.

@bryanhuntesl
Copy link
Contributor Author

bryanhuntesl commented Nov 13, 2018

Just been looking at confex with Lazslo H - I wonder if it would make a good solution to config from ENV (with defaults) - might be better than rolling our own. Ping on hipchat @sescobb27 @filipevarjao

@bryanhuntesl bryanhuntesl merged commit edb7e1a into master Nov 15, 2018
@bryanhuntesl bryanhuntesl deleted the feature/travis-initial branch November 15, 2018 11:20
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.

None yet

2 participants