DEV 2005 add gitlab ci #409
Conversation
setup.py
Outdated
| "notebook", | ||
| "jupyter", | ||
| "jupyter-client", | ||
| "jupyter-console", | ||
| "jupyter-core", |
There was a problem hiding this comment.
Do we really need all these for "dev" (and testing)?
Maybe add one more, "jupyter"?
Another alternative would be to add "test" and only include pytest-related deps. This way, tox would install ".[test]" (assuming tests don't need jupyter stuff) and save some time.
It's ok as is. It's just that "dev" with everything will add bootstrap time to each tox build (each commit).
test/conftest.py
Outdated
| @@ -7,7 +7,7 @@ | |||
| import random | |||
| import unittest | |||
| import uuid | |||
| from test.helpers import create_tables, truncate | |||
| from test.helpers import create_tables, truncate, DB_CONFIG | |||
There was a problem hiding this comment.
This is an optional suggestion. The google style guide recommends not importing specific functions or classes. We as a team tend to relax that a bit for classes and configs but I think for functions it's still a preference. I think the preference here would be to do from test import helpers and then add the the dot notations. This is existing code but since we're updating the line it may make sense to bring it within standards.
test/helpers.py
Outdated
| "user": os.getenv("PG_HOST", "test"), | ||
| "password": os.getenv("PG_PASS", "test"), | ||
| "database": os.getenv("PG_NAME","automated_test"), | ||
| } |
test/test_admin_script.py
Outdated
| @@ -6,9 +6,11 @@ | |||
| from gdcdatamodel import gdc_postgres_admin as pgadmin | |||
| from gdcdatamodel import models | |||
|
|
|||
| from test.helpers import DB_CONFIG | |||
There was a problem hiding this comment.
Optional suggestion for this and the imports in the other test files.
| from test.helpers import DB_CONFIG | |
| from test.helpers import DB_CONFIG |
Should wait for this pr to be merged and update the pin:
https://github.com/NCI-GDC/base-containers/pull/28