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

fix(build): content-type fails when deployed within internal network controls #128

Merged
merged 12 commits into from
Jan 25, 2022

Conversation

m3mike
Copy link
Contributor

@m3mike m3mike commented Jan 20, 2022

What Changed

  • Moved ML training to container build time (previously at container startup)
  • Added nginx as reverse proxy in Docker
    • Nginx handles static file delivery
    • Embedded nginx configuration in new tram-nginx container image to work in docker compose
    • Updated CI to build nginx container with embedded configuration

Minor

  • Excluded tox files from container image
  • Updated README link to always point to latest release (previously release version was hardcoded)
  • Removed hardcoded Django SECRET_KEY, key is now autogenerated on application startup for session security

Limitations

  • Model training now done in Dockerfile, meaning model changes will require a new container image.
  • Still using sqlite, which will be bottleneck if deployed as-is, may want to switch to postgresql depending on use case
  • As Django secret key is now autogenerated on startup, if application is restarted/reloaded, session key will be regenerated and users will need to login again. To go back to the old behavior, set a static SECRET_KEY value in the docker/docker-compose.yml file.

@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #128 (1e83d66) into master (7e009d8) will increase coverage by 0.12%.
The diff coverage is 88.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   94.43%   94.56%   +0.12%     
==========================================
  Files          20       20              
  Lines         827      828       +1     
==========================================
+ Hits          781      783       +2     
+ Misses         46       45       -1     
Impacted Files Coverage Δ
src/tram/tram/settings.py 92.85% <88.00%> (+2.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e009d8...1e83d66. Read the comment docs.

@m3mike m3mike changed the title fix(build): content-type fails when deployed within internal network controls WIP - fix(build): content-type fails when deployed within internal network controls Jan 20, 2022
* prevents having to hardcode value in docker-compose, value will now be unique to each deployment
* allows embedding of configuration to simplify end user usability
* Add CI trigger for new container
* Add Dockerfile for new container (Dockerfile.nginx)
* Update docker-compose.yml to build locally if images not available upstream
* Reformat README a bit for readability
* Update Makefile to build new nginx container
@m3mike m3mike changed the title WIP - fix(build): content-type fails when deployed within internal network controls fix(build): content-type fails when deployed within internal network controls Jan 21, 2022
@m3mike m3mike requested a review from markeaimark January 21, 2022 17:12
* simplified runtime by using gunicorn worker, no async code so no app impact (removed uvicorn from deps)
* added protocol scheme (http/https) to forwarded headers for nginx
@m3mike m3mike requested a review from markeaimark January 25, 2022 18:36
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@markeaimark markeaimark merged commit 37f2846 into master Jan 25, 2022
@markeaimark markeaimark deleted the fix/set-content-type branch January 25, 2022 18:43
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.

2 participants