hackforla / HomeUniteUs

We're working with community non-profits who have a Host Home or empty bedrooms initiative to develop a workflow management tool to make the process scalable (across all providers), reduce institutional bias, and effectively capture data.
https://homeunite.us/
GNU General Public License v2.0
36 stars 21 forks source link

refactor(api): Prepare for EC2 deployment #544

Closed paulespinosa closed 9 months ago

paulespinosa commented 10 months ago

The reason for these changes is to prepare the code with the ability to build a versioned package that can be deployed to a EC2 instance. The build will be done by CI and the package uploaded to the EC2 instance.

Related #465

paulespinosa commented 10 months ago

The reason I migrated to pyproject.toml was because I saw there is a "setuptools" extension that can automatically version the API based on git hashes and tags.

But this might make a breaking change to your development workflow because there is a slightly different way to get dependencies installed. If you want to minimize any possible disruption, then I'll just keep the change to __main__.py for now. Let me know.

ju1es commented 10 months ago

The reason I migrated to pyproject.toml was because I saw there is a "setuptools" extension that can automatically version the API based on git hashes and tags.

But this might make a breaking change to your development workflow because there is a slightly different way to get dependencies installed. If you want to minimize any possible disruption, then I'll just keep the change to __main__.py for now. Let me know.

thanks for tackling the prereqs for ec2! couple questions,

  1. "version the API based on git hashes and tags", what's the value added here? sorry i'm not understanding it right away but can you give an example.
  2. Might break the dev workflow, would this affect folks that already have their dependencies setup? I'm all for wheels but wondering what a smoother transition would be like.
paulespinosa commented 10 months ago

The reason I migrated to pyproject.toml was because I saw there is a "setuptools" extension that can automatically version the API based on git hashes and tags. But this might make a breaking change to your development workflow because there is a slightly different way to get dependencies installed. If you want to minimize any possible disruption, then I'll just keep the change to __main__.py for now. Let me know.

thanks for tackling the prereqs for ec2! couple questions,

1. "version the API based on git hashes and tags", what's the value added here? sorry i'm not understanding it right away but can you give an example.

2. Might break the dev workflow,  would this affect folks that already have their dependencies setup? I'm all for wheels but wondering what a smoother transition would be like.

Hi @ju1es !

  1. We can use the version to track down when a feature or bug was introduced to the code and why. If we continue the practice of writing down features and bugs in issues and then referencing the issues in code commits and PRs, then we can be able to understand why the program is running as it is. Another helpful practice that we'll eventually get to is tagging and branching releases. Since we aren't practicing continuous deployment, our main branch may not reflect what is running on our EC2 instance (or other environment we have in the future).

  2. The wheel part of this is mainly for deployment which is being automated by a GitHub workflow. The only (that I'm currently aware of) differing command we developers need to run is pip install -e ".[dev]" instead of pip install -r requirements.txt. It'd be great if you or anyone else has spare time to check this branch out, give it a try, and provide feedback.

Thank you!

paulespinosa commented 10 months ago

Try this: pip install ".[prod]". It'll install gunicorn along with the api.

Joshua-Douglas commented 10 months ago

Try this: pip install ".[prod]". It'll install gunicorn along with the api.

Hey @paulespinosa,

Great point, corrected in 7a90fc9.

I also took the time to run the install instructions on a clean virtual environment, and I ran into several errors. __main__.py had a minor typo and several requirements were missing from the dependencies list. I also had to run pip install --upgrade pip to avoid some build errors. See 8855d63 and eedbe0c for more detail.

paulespinosa commented 10 months ago

@Joshua-Douglas which of those dependencies are transitive dependencies? Let's keep the list of dependencies to be the direct dependencies required.

Joshua-Douglas commented 10 months ago

Hey @paulespinosa, I'm not sure. I tried to determine by running the pipreqs tools but it generated a list as long as the previous requirements.txt file. I tried adding two dependencies but kept running into errors. I think finding the true list of direct dependencies will take some work.

I know for sure that prance and openapi-spec-validator were needed. After I added those two I was still running into errors, so I restored the previous requirements.txt. You are welcome to trim it though.

paulespinosa commented 10 months ago

@Joshua-Douglas what were the errors?

paulespinosa commented 10 months ago

Few weeks ago we used poetry to analyze the dependency tree.

Joshua-Douglas commented 10 months ago

Few weeks ago we used poetry to analyze the dependency tree.

Hey @paulespinosa, @ju1es said he would take a look at fixing this. If poetry can reduce the dependencies list, that would be great!

I can also take another look if that helps.

ju1es commented 10 months ago

I uninstalled all deps in my env via,

pip freeze > uninstall.txt
pip uninstall -y -r uninstall.txt
(is this the proper way??)

then ran pip install ".[dev]" and that completed with no errors.

pip freeze at this point gives me,

alembic==1.11.1
attrs==23.1.0
boto3==1.22.10
botocore==1.25.13
certifi==2023.7.22
charset-normalizer==3.2.0
click==8.1.6
clickclick==20.10.2
connexion==2.14.2
Flask==2.2.5
gunicorn==21.2.0
idna==3.4
inflection==0.5.1
itsdangerous==2.1.2
Jinja2==3.1.2
jmespath==1.0.1
jsonschema==4.18.4
jsonschema-specifications==2023.7.1
Mako==1.2.4
MarkupSafe==2.1.3
openapi-server @ file:///Users/jules/hack_for_la/HomeUniteUs/api
packaging==23.1
psycopg2-binary==2.9.6
python-dateutil==2.8.2
python-dotenv==1.0.0
PyYAML==6.0.1
referencing==0.30.0
requests==2.31.0
rpds-py==0.9.2
s3transfer==0.5.2
six==1.16.0
SQLAlchemy==2.0.19
swagger-ui-bundle==0.0.9
typing_extensions==4.7.1
urllib3==1.26.16
Werkzeug==2.2.3

which are the transitive + direct deps right? and the .toml direct deps are still,

dependencies = [
    "alembic==1.11.1",
    "boto3==1.22.10",
    "connexion [swagger-ui]==2.14.2",
    "psycopg2-binary==2.9.6",
    "python-dotenv==1.0.0",
]

Same thing with prod. Lmk if I'm missing something?

paulespinosa commented 10 months ago

I uninstalled all deps in my env via,

pip freeze > uninstall.txt
pip uninstall -y -r uninstall.txt
(is this the proper way??)

Yes that's how I know to do it, too. The other is to create a new virtual environment.

then ran pip install ".[dev]" and that completed with no errors.

pip freeze at this point gives me,

alembic==1.11.1
attrs==23.1.0
boto3==1.22.10
botocore==1.25.13
certifi==2023.7.22
charset-normalizer==3.2.0
click==8.1.6
clickclick==20.10.2
connexion==2.14.2
Flask==2.2.5
gunicorn==21.2.0
idna==3.4
inflection==0.5.1
itsdangerous==2.1.2
Jinja2==3.1.2
jmespath==1.0.1
jsonschema==4.18.4
jsonschema-specifications==2023.7.1
Mako==1.2.4
MarkupSafe==2.1.3
openapi-server @ file:///Users/jules/hack_for_la/HomeUniteUs/api
packaging==23.1
psycopg2-binary==2.9.6
python-dateutil==2.8.2
python-dotenv==1.0.0
PyYAML==6.0.1
referencing==0.30.0
requests==2.31.0
rpds-py==0.9.2
s3transfer==0.5.2
six==1.16.0
SQLAlchemy==2.0.19
swagger-ui-bundle==0.0.9
typing_extensions==4.7.1
urllib3==1.26.16
Werkzeug==2.2.3

which are the transitive + direct deps right? and the .toml direct deps are still,

dependencies = [
    "alembic==1.11.1",
    "boto3==1.22.10",
    "connexion [swagger-ui]==2.14.2",
    "psycopg2-binary==2.9.6",
    "python-dotenv==1.0.0",
]

Same thing with prod. Lmk if I'm missing something?

The approach looks correct. We have to add two new dependencies though that were added in main branch.

prance==23.6.21.0 openapi-spec-validator==0.6.0

One thing to note use that in this PR we upgraded the direct deps versions and Python version so the pip freeze list is going to look different than what's the current main branch.

Joshua-Douglas commented 10 months ago

@Joshua-Douglas what were the errors?

@paulespinosa, @ju1es,

I realized the error was occurring while I was running the test cases since Flask-Testing was missing. So there were only three dependencies missing: prance, openapi-spec-validator, and Flask-Testing.

I rolled back my brute force dependency update in f13dba0 and replaced it with just those three dependencies in 9d7b7b3.

In my update I did not chose a specific version, just to make dependency updates easier in the future. If you'd prefer that we tie ourselves to exact versions, just let me know.

Joshua-Douglas commented 10 months ago

then ran pip install ".[dev]" and that completed with no errors.

Hey @ju1es,

I was only seeing the issues while running the code after installing the dependencies. So running gunicorn -w 4 "openapi_server.__main__:create_app()" or pytest gave me the error.

I took another look at the dependencies, and think I added them all now.

paulespinosa commented 10 months ago

@Joshua-Douglas @ju1es Thank you for helping out on this one. I added some documentation to pyproject.toml.

It looks like this can be merged with main without conflicts (as of this message).

ju1es commented 10 months ago

@Joshua-Douglas what were the errors?

@paulespinosa, @ju1es,

I realized the error was occurring while I was running the test cases since Flask-Testing was missing. So there were only three dependencies missing: prance, openapi-spec-validator, and Flask-Testing.

I rolled back my brute force dependency update in f13dba0 and replaced it with just those three dependencies in 9d7b7b3.

In my update I did not chose a specific version, just to make dependency updates easier in the future. If you'd prefer that we tie ourselves to exact versions, just let me know.

maybe we just pin to the major version?

ju1es commented 10 months ago

this seems like a flopping test. but approving. thank yall @Joshua-Douglas @paulespinosa for sorting out the dependency management 🙇

Joshua-Douglas commented 10 months ago

Hey @paulespinosa,

As a heads up, I merged with main and make some modifications to the Dockerfile because it was not building:

  1. You removed FROM node:latest as swagger-bundle from the API dockerfile in dffec740810f1b35f665265f46b826e0c1788945, but you left the call to COPY --from=swagger-bundle /usr/share/misc/openapi.yaml openapi_server/_spec/openapi.yaml. The swagger-bundle build stage no longer exists, so I removed that call to COPY. It shouldn't be necessary since we are already copying the entire API directory into the image.
  2. Remove tox.ini from the .dockerignore. It is necessary to add the tox configuration into the container if we want to run the test cases within the container using tox. With this change you can use docker exec tox, as outlined in the README.

It looks like the versioning strategy is still being developed since PSEDUO_VERSION is not really being used anywhere. Are you planning on submitting a separate PR, or would you like some help with this?

Thanks! Josh

paulespinosa commented 9 months ago

It looks like the versioning strategy is still being developed since PSEDUO_VERSION is not really being used anywhere.

PSEDUO_VERSION is used on the next line in Dockerfile. The variable it sets on the next line controls the setuptools-scm tool to give the package a version.

paulespinosa commented 9 months ago

@ju1es @Joshua-Douglas Requirements file have been re-introduced after inspecting the core dependencies. The pip-compile tool from the pip-tools project was used to generate two requirements files based on the core dependencies listed in pyproject.toml:

This was done after discussion with Jules and acknowledging the value of using pinned versions of all core and transitive dependencies used by each environment throughout the API's release lifecycle.

Now, we are back to installing the requirements file. For example, pip install -r requirements-dev.txt instead of pip install ".[dev]". (Production is a bit different and documented in the README).