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

Add flower to dev dependencies #2958

Merged

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Feb 17, 2021

Description

Add Flower to dev dependencies and mention that we have it available in README.

@aronasorman When I run pip-compile, it also changed other dependencies' format (maybe it's because I am using the newest version?) Is it okay? To make the review easier, I run pip-compile on the original requirements-dev.in without adding any new updates and committed the diff. Then I added the flower package to requirements, recompiled them, and committed this change in another commit.

Issue Addressed

I hope this will make locating this tool easier for developers who are not so familiar with backend tooling like me. Also, I noticed that we actually run flower already as part of services command so I assumed that it would work, and it took me a while to figure out that it wasn't working just due to a missing dependency.

Steps to Test

  • Run pip install -r requirements-dev.txt
  • Run yarn run services
  • See Flower dashboard on http://localhost:5555

@MisRob MisRob force-pushed the add-flower-to-dev-dependencies branch from f0a9e26 to 07a4cdc Compare February 17, 2021 08:51
@@ -39,3 +39,4 @@ pypandoc
git+https://github.com/someshchaturvedi/customizable-django-profiler.git#customizable-django-profiler
tabulate==0.8.2
fonttools
flower==0.9.4
Copy link
Member Author

Choose a reason for hiding this comment

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

I specified this version because when I didn't do it, I was getting this error during compilation:

Tried: 0.0.1, 0.0.2, 0.0.3, 0.0.4, 0.0.5, 0.0.6, 0.0.7, 0.0.8, 0.0.9, 0.0.10, 0.0.11, 0.0.12, 0.0.13, 0.0.14, 0.0.15, 0.0.16, 0.0.17, 0.0.18, 0.0.19, 0.0.20, 0.0.21, 0.1.0, 0.1.1, 0.2.0, 0.3.0, 0.3.1, 0.4.0, 0.4.1, 0.4.2, 0.5.0, 0.6.0, 0.7.0, 0.7.1, 0.8.0, 0.8.0, 0.9.0, 0.9.0
There are incompatible versions in the resolved dependencies:
  prometheus-client==0.7.1 (from -c requirements.txt (line 66))
  prometheus-client==0.8.0 (from flower==0.9.7->-r requirements-dev.in (line 42))

I didn't want to touch production dependencies in this PR. Flower 0.9.4 is the newest version that worked well with our current dependencies. Is this approach okay?

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely.

@MisRob
Copy link
Member Author

MisRob commented Feb 17, 2021

Frontend linting check failure is fixed in #2957

@MisRob MisRob requested a review from marcellamaki February 17, 2021 09:00
@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #2958 (f0a9e26) into develop (6cd38c6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2958   +/-   ##
========================================
  Coverage    85.53%   85.53%           
========================================
  Files          299      299           
  Lines        15827    15827           
========================================
  Hits         13538    13538           
  Misses        2289     2289           

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 f9defac...b0af1d7. Read the comment docs.

@rtibbles rtibbles merged commit 61066c0 into learningequality:develop Feb 17, 2021
@MisRob MisRob deleted the add-flower-to-dev-dependencies branch March 15, 2021 12:35
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