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 versions of python modules not being set in docker images #977

Merged
merged 33 commits into from
Jul 1, 2019

Conversation

greenape
Copy link
Member

Closes #818

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

Description

Uses versioneer's ability to infer a version number from a source tree to set the version numbers for python modules inside docker containers.

@greenape greenape added bug Something isn't working FlowAuth Issues related to FlowAuth FlowMachine Issues related to FlowMachine FlowAPI Issues related to the FlowKit API ready-to-merge Label indicating a PR is OK to automerge docker worked_examples labels Jun 25, 2019
@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #977 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #977   +/-   ##
=======================================
  Coverage   93.33%   93.33%           
=======================================
  Files         136      136           
  Lines        6861     6861           
  Branches      697      697           
=======================================
  Hits         6404     6404           
+ Misses        333      332    -1     
- Partials      124      125    +1
Flag Coverage Δ
#flowapi_unit_tests 82.98% <ø> (+5.07%) ⬆️
#flowauth_unit_tests 94.86% <ø> (ø) ⬆️
#flowclient_unit_tests 82.6% <ø> (ø) ⬆️
#flowetl_unit_tests 100% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.09% <ø> (-0.04%) ⬇️
#integration_tests 59.83% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
flowmachine/flowmachine/models/louvain.py 91.42% <0%> (-5.72%) ⬇️
flowmachine/flowmachine/core/cache.py 95.23% <0%> (-0.53%) ⬇️
flowapi/flowapi/main.py 100% <0%> (+3.89%) ⬆️

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 f8f6030...7c715a9. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #977 into master will increase coverage by 1.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #977      +/-   ##
==========================================
+ Coverage   92.77%   93.92%   +1.15%     
==========================================
  Files         140      140              
  Lines        6902     6902              
  Branches      693      693              
==========================================
+ Hits         6403     6483      +80     
+ Misses        380      308      -72     
+ Partials      119      111       -8
Flag Coverage Δ
#flowapi_unit_tests 82.4% <ø> (+5.13%) ⬆️
#flowauth_unit_tests 94.86% <ø> (ø) ⬆️
#flowclient_unit_tests 81.99% <ø> (?)
#flowetl_unit_tests 100% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.48% <ø> (ø) ⬆️
#integration_tests 61.54% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
flowapi/flowapi/main.py 100% <0%> (+3.89%) ⬆️
flowclient/flowclient/client.py 93.83% <0%> (+36.49%) ⬆️

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 6bcbe11...d57487d. Read the comment docs.

Dockerfile Outdated Show resolved Hide resolved
@@ -382,7 +382,7 @@ def render_pep440_post(pieces):

The ".dev0" means dirty. Note that .dev0 sorts backwards
(a dirty tree will appear "older" than the corresponding clean one),
but you shouldn't be releasing software with -dirty anyway.
but you shouldn't be releasing software with -dirty anyways.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

RUN python setup.py bdist_wheel && pip install dist/*.whl
WORKDIR /app
COPY backend/uwsgi.ini .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the purpose of this uwsgi.ini file? Should this be included in the distribution instead (e.g. via MANIFEST.in)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It configures uwsgi, which is serving the app in combination with nginx. So is deployment-side rather than distribution side.

Copy link
Contributor

@maxalbert maxalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@mergify mergify bot merged commit 0e52291 into master Jul 1, 2019
@mergify mergify bot deleted the versioning branch July 1, 2019 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docker FlowAPI Issues related to the FlowKit API FlowAuth Issues related to FlowAuth FlowMachine Issues related to FlowMachine ready-to-merge Label indicating a PR is OK to automerge worked_examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 0+unknown
2 participants