Remove /opt/graphite prefix and use setuptools #2409

piotr1212 · 2019-01-08T12:43:02Z

See graphite-project/carbon#835 for reasoning

codecov-io · 2019-01-08T12:54:11Z

Codecov Report

Merging #2409 into master will decrease coverage by 0.02%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #2409      +/-   ##
==========================================
- Coverage   80.26%   80.24%   -0.03%     
==========================================
  Files          87       87              
  Lines        9323     9333      +10     
  Branches     1983     1984       +1     
==========================================
+ Hits         7483     7489       +6     
+ Misses       1558     1557       -1     
- Partials      282      287       +5

Impacted Files	Coverage Δ
webapp/graphite/logger.py	`87.03% <69.23%> (-5.83%)`	⬇️
webapp/graphite/settings.py	`74.75% <100.00%> (-0.25%)`	⬇️
webapp/graphite/dashboard/views.py	`98.51% <0.00%> (-0.75%)`	⬇️
webapp/graphite/wsgi.py	`40.62% <0.00%> (ø)`
webapp/graphite/app_settings.py	`70.96% <0.00%> (ø)`
webapp/graphite/tags/base.py	`91.79% <0.00%> (+1.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 94aad8f...42cc6d2. Read the comment docs.

webapp/graphite/settings.py

piotr1212 · 2019-02-02T20:47:26Z

I'm not finished yet but made some changes to the docs which I would like to get some feedback for. I've tried to simplify the docs, IMO there were too much separate pages which you had to jump back and forward to.

I've changed the default install (settings.py) so that running collectstatic is not needed. The static files can be served directly from the app with whitenoise. Serving from whitenoise should be fast enough for most installations. this eliminates the need for configuring the static dir in the webserver (simplifies installation). From what I've read the whole purpose of collectstatic is for organisations which run multiple django apps and have separated their static files from code (in repo), so that they can update static files without having to deploy code and vice versa. As graphite's static files haven't changed in years and they are in the code repo I don't see a point to require collectstatic in the default install. Users can still run collectstatic if they want/need.

webapp/graphite/local_settings.py.example

setup.py

ploxiln · 2019-02-13T04:23:11Z

I think a rebase went a bit wrong, you ended up with a copy of the commit "fix dashboard graph metric list icon paths with URL_PREFIX" from the master branch
in master: 0a037db
in this branch: 1ba4da5

webapp/graphite/logger.py

Don't install in /opt/graphite anymore, this is just strange and causes nothing but trouble. Setuptools does not support it either. Switch to setuptools to get rid of following warnings: UserWarning: Unknown distribution option: 'install_requires' UserWarning: Unknown distribution option: 'long_description_content_type'

Synthesize is not being updated anymore (latest version is 1.0.2). Removing it from the docs to not confuse users. And we have an official docker image now.

The GRAPHITE_ROOT was to be set to the parent directory of the path where graphite-web is installed. The `storage`, `static` and `content` directories are set to be dubdirs of GRAPHITE_ROOT. If installing Graphite in the regular Python path's this would make the directories look something like `/usr/lib/python3.7/{storage,static,conf}`. Which is probably not what the user wants. This sets the default to `sys.prefix`. sys.prefix

Installing the static assets in the project directory instead of an external directory make installation easier. Add whitenoise to requires in setup.py so that the static assets are automatically (without user action) served after installation. Remove package_data from setup.py, you should use package_data or MANIFEST.in to specify which non-python files have to be installed, not both.

…_settings.py Fixes dashboards which were broken with last commit

Fix some spelling

piotr1212 · 2019-02-20T17:22:49Z

I think a rebase went a bit wrong, you ended up with a copy of the commit "fix dashboard graph metric list icon paths with URL_PREFIX" from the master branch
in master: 0a037db
in this branch: 1ba4da5

I think I merged instead of rebased. Anyway, cleaned up now.

ploxiln · 2019-02-20T19:56:46Z

docs/releases/1_2_0.rst

+
+Carbonate
+^^^^^^^^^
+kA None


very minor - this "kA" looks unintentional

ploxiln · 2019-02-20T20:06:27Z

All looks good to me.

I had another thought about storage dirs ... I think the original idea behind using /opt/graphite is those storage and log dirs, which would be awkward in the python site-packages directory. Maybe the thing to do is divorce them from the graphite application root, and default to /opt/graphite/storage and /opt/graphite/log regardless of the install prefix? And not mention them in setup.py at all? I suppose the downside is they would not be created by install. Just an idea - I suppose I always customize these dirs anyway.

piotr1212 · 2019-02-20T21:12:11Z

good point. I'll have a look. But busy at the moment with more higher prio stuff.

stale · 2020-04-13T20:33:44Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ploxiln · 2020-04-13T21:00:09Z

This is a big painful change, but I think it's still relatively important. Just sayin' to appease the stale bot :)

adamboutcher · 2023-07-19T16:36:01Z

If this will fix the pip install, can we get it merged?

deniszh · 2023-07-19T17:53:43Z

Unfortunately, it's not that easy. That would break backward compatibility, and need more changes in carbon. Also, not sure if that fix issue too :/ ср, 19 июл. 2023 г., 18:36 Adam ***@***.***>:

…

If this will fix the pip install, can we get it merged? — Reply to this email directly, view it on GitHub <#2409 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJLTVRNEHAUAT6W7NWQH7LXRAEH3ANCNFSM4GOVTH5Q> . You are receiving this because your review was requested.Message ID: ***@***.***>

piotr1212 requested a review from deniszh January 8, 2019 12:43

piotr1212 force-pushed the setuptools branch from e1aed95 to 3c02110 Compare January 31, 2019 20:48

piotr1212 mentioned this pull request Feb 1, 2019

FIX /static/ alias for Apache config #2421

Closed

piotr1212 commented Feb 2, 2019

View reviewed changes

webapp/graphite/settings.py Show resolved Hide resolved

piotr1212 force-pushed the setuptools branch from fc15b43 to cbf6dc3 Compare February 3, 2019 19:43

ploxiln reviewed Feb 6, 2019

View reviewed changes

webapp/graphite/local_settings.py.example Outdated Show resolved Hide resolved

ploxiln mentioned this pull request Feb 7, 2019

docs: for sql db migration to 1.1 recommend --fake-initial #2425

Merged

deniszh approved these changes Feb 7, 2019

View reviewed changes

piotr1212 force-pushed the setuptools branch 2 times, most recently from 5b1cd5e to 1ba4da5 Compare February 7, 2019 10:35

ploxiln reviewed Feb 13, 2019

View reviewed changes

setup.py Outdated Show resolved Hide resolved

ploxiln reviewed Feb 13, 2019

View reviewed changes

webapp/graphite/logger.py Show resolved Hide resolved

piotr1212 mentioned this pull request Feb 19, 2019

Graphite Web composer does not load fully, static files missing. #2427

Closed

piotr1212 added 8 commits February 20, 2019 17:06

Remove synthesize from installation documentation

41a7eda

Synthesize is not being updated anymore (latest version is 1.0.2). Removing it from the docs to not confuse users. And we have an official docker image now.

setup.py: update supported django version (2.1)

f6e5792

logger.py: create directory if not exists

f44d8b0

Remove STATICFILES_DIRS from settings.py and add explanation in local…

ce76c4a

…_settings.py Fixes dashboards which were broken with last commit

Update and simplify docs

15185fa

Docs: Add upgrade instructions

caed07b

Fix some spelling

piotr1212 force-pushed the setuptools branch from 1ba4da5 to caed07b Compare February 20, 2019 16:22

cleanup setup.py and logger.py

de040e2

ploxiln reviewed Feb 20, 2019

View reviewed changes

docs/releases/1_2_0.rst

Carbonate

^^^^^^^^^

kA None

Copy link

Contributor

ploxiln Feb 20, 2019

Choose a reason for hiding this comment

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

very minor - this "kA" looks unintentional

piotr1212 added 2 commits July 5, 2019 10:07

Merge branch 'django22' into setuptools

ac582bc

Merge remote-tracking branch 'origin/master' into setuptools

fc6481f

nook24 mentioned this pull request Oct 2, 2019

All static files like ext-all.js are missing this leads to a blank page #2492

Closed

ploxiln mentioned this pull request Mar 15, 2020

virtualenv 20.0.x changes break default layout install #2566

Open

stale bot added the stale label Apr 13, 2020

stale bot removed the stale label Apr 13, 2020

ploxiln mentioned this pull request Apr 13, 2020

Remove /opt/graphite prefix and use setuptools graphite-project/carbon#835

Open

piotr1212 added 4 commits April 27, 2020 21:21

Merge remote-tracking branch 'origin/master' into setuptools

424545b

pin whitenoise < 5

e18778d

Merge remote-tracking branch 'origin/master' into setuptools

785a8f7

Keep /opt/graphite as default GRAPHITE_ROOT

42cc6d2

ploxiln mentioned this pull request May 20, 2020

Lock whitenoise dependency version #2606

Merged

deniszh added the pinned label May 20, 2020

nook24 mentioned this pull request Dec 9, 2022

[Q] Issues with Python 3.10 whitenoise 4 and Graphite Web 1.1.10 #2791

Closed

adamboutcher mentioned this pull request Jul 19, 2023

[BUG] Cant install using pip. #2815

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove /opt/graphite prefix and use setuptools #2409

Remove /opt/graphite prefix and use setuptools #2409

piotr1212 commented Jan 8, 2019

codecov-io commented Jan 8, 2019 •

edited

Loading

piotr1212 commented Feb 2, 2019 •

edited

Loading

ploxiln commented Feb 13, 2019

piotr1212 commented Feb 20, 2019

ploxiln Feb 20, 2019

ploxiln commented Feb 20, 2019

piotr1212 commented Feb 20, 2019

stale bot commented Apr 13, 2020

ploxiln commented Apr 13, 2020

adamboutcher commented Jul 19, 2023

deniszh commented Jul 19, 2023 via email

Remove /opt/graphite prefix and use setuptools #2409

Are you sure you want to change the base?

Remove /opt/graphite prefix and use setuptools #2409

Conversation

piotr1212 commented Jan 8, 2019

codecov-io commented Jan 8, 2019 • edited Loading

Codecov Report

piotr1212 commented Feb 2, 2019 • edited Loading

ploxiln commented Feb 13, 2019

piotr1212 commented Feb 20, 2019

ploxiln Feb 20, 2019

Choose a reason for hiding this comment

ploxiln commented Feb 20, 2019

piotr1212 commented Feb 20, 2019

stale bot commented Apr 13, 2020

ploxiln commented Apr 13, 2020

adamboutcher commented Jul 19, 2023

deniszh commented Jul 19, 2023 via email

codecov-io commented Jan 8, 2019 •

edited

Loading

piotr1212 commented Feb 2, 2019 •

edited

Loading