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

Cli conversion (fix #189) #310

Open
wants to merge 49 commits into
base: master
from
Open

Cli conversion (fix #189) #310

wants to merge 49 commits into from

Conversation

@borrob
Copy link
Collaborator

borrob commented Jan 8, 2020

This PR brings an unified CLI to control GeoHealthCheck (fix #189).

I introduced click to take control over all the cli-commands and I was able to completely phase out paver (paver is still available, but I marked it as deprecated). To keep an overview, I kept all of the cli commands in the file geohc_cli.py. I also introduced a setup.py file in order to install GHC with a simple pip install .. I believe we can now also upload it to pypi.

@tomkralidis : can you review the setup.py file since I assumed you would be the maintainer.

The CLI command now allow for: geohc create-instance, geohc db-create, geohc db-adduser -u rob -p mypass -e e@mail.com -r admin, and geohc --help.

borrob and others added 30 commits Dec 11, 2019
The new CLI will call commands from outside the package. The import statements
needed to be reworked. Not proud of the solution, but I cannot think of
something better now.
With the setup file we can install the cli command `geohc` and build eggs and
wheels.
Move all the models.py ... commands to the geohc cli and add deprecation
warnings.
Need to fix loose ends:
- DB exists, but still needs db-create
- check import statements
- fix env setting for GHC_SETTINGS_CONF
- check non-docker build
- update docs
TODO: test the create-instance, double check os.path things
@borrob borrob requested a review from tomkralidis Jan 8, 2020
borrob added 5 commits Jan 8, 2020
@justb4 justb4 added the enhancement label Jan 9, 2020
@justb4 justb4 added this to the Version 0.8.0 milestone Jan 9, 2020
@justb4

This comment has been minimized.

Copy link
Member

justb4 commented Jan 9, 2020

@borrob again great effort! See you even thought of Docker build. @tomkralidis will you be able to review?

.travis.yml Show resolved Hide resolved
GeoHealthCheck/__init__.py Outdated Show resolved Hide resolved
GeoHealthCheck/geohc_cli.py Outdated Show resolved Hide resolved
GeoHealthCheck/models.py Outdated Show resolved Hide resolved
@@ -1,5 +1,3 @@
Paver==1.3.4
Sphinx==1.8.1

This comment has been minimized.

Copy link
@tomkralidis

tomkralidis Jan 12, 2020

Member

What happens to our Sphinx build if we remove this dependency?

This comment has been minimized.

Copy link
@borrob

borrob Jan 13, 2020

Author Collaborator

It should still work, since pip install -e . will install Sphinx because it is in requirements.txt

docs/admin.rst Outdated Show resolved Hide resolved
@@ -15,3 +15,4 @@ requests>=2.20.0
WTForms==2.2.1
APScheduler==3.6.1
passlib==1.7.1
paver==1.3.4

This comment has been minimized.

Copy link
@tomkralidis

tomkralidis Jan 12, 2020

Member

Why don't we remove Paver and pavement.py alltogether?

This comment has been minimized.

Copy link
@borrob

borrob Jan 13, 2020

Author Collaborator

I managed to convert all cli commands to click, so there is no technical reason to keep paver.

I don't know if other people made their own docker scripts/workflow and I thought it would be good to keep it around, but mark it as deprecated so we can remove it the next version.

This comment has been minimized.

Copy link
@tomkralidis

tomkralidis Jan 13, 2020

Member

In this case folks can use an earlier version of GHC no?

This comment has been minimized.

Copy link
@justb4

justb4 Jan 14, 2020

Member

I would suggest to remove Paver.

This comment has been minimized.

Copy link
@borrob

borrob Jan 15, 2020

Author Collaborator

I removed paver with commit 57383c0

setup.py Show resolved Hide resolved
setup.py Outdated
'Services and web APIs.'

setup(
name='geohealthcheck',

This comment has been minimized.

Copy link
@tomkralidis

tomkralidis Jan 12, 2020

Member

GeoHealthCheck

This comment has been minimized.

Copy link
@borrob

borrob Jan 13, 2020

Author Collaborator

Done. Commit: f6da999

@borrob borrob mentioned this pull request Jan 15, 2020
borrob added 2 commits Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.