graphite-web
graphite-web copied to clipboard
Remove /opt/graphite prefix and use setuptools
See https://github.com/graphite-project/carbon/pull/835 for reasoning
Codecov Report
Merging #2409 into master will decrease coverage by
0.02%
. The diff coverage is75.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%) |
:arrow_down: |
webapp/graphite/settings.py | 74.75% <100.00%> (-0.25%) |
:arrow_down: |
webapp/graphite/dashboard/views.py | 98.51% <0.00%> (-0.75%) |
:arrow_down: |
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%) |
:arrow_up: |
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.
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.
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: 0a037db4b2d864734e14dd6302bc71194f53e8d3 in this branch: 1ba4da55c08035cccfcdaae2220f8d384dbd1929
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.
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.
good point. I'll have a look. But busy at the moment with more higher prio stuff.
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.
This is a big painful change, but I think it's still relatively important. Just sayin' to appease the stale bot :)
If this will fix the pip install, can we get it merged?
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 https://github.com/graphite-project/graphite-web/pull/2409#issuecomment-1642409417, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJLTVRNEHAUAT6W7NWQH7LXRAEH3ANCNFSM4GOVTH5Q . You are receiving this because your review was requested.Message ID: @.***>