mesa icon indicating copy to clipboard operation
mesa copied to clipboard

Feature urlpath in ModularServer

Open Holzhauer opened this issue 4 years ago • 9 comments

When deploying several models on a public server behind a proxy, adding a url path segment is required, e.g. http://server/mesa-schelling > localhost:8521/mesa-schelling

Holzhauer avatar Apr 28 '22 11:04 Holzhauer

Codecov Report

Merging #1296 (ccc7702) into main (736367a) will decrease coverage by 0.12%. The diff coverage is 57.14%.

:exclamation: Current head ccc7702 differs from pull request most recent head b10fe9f. Consider uploading reports for the commit b10fe9f to get more accurate results

@@            Coverage Diff             @@
##             main    #1296      +/-   ##
==========================================
- Coverage   91.15%   91.02%   -0.13%     
==========================================
  Files          18       18              
  Lines        1357     1360       +3     
  Branches      261      262       +1     
==========================================
+ Hits         1237     1238       +1     
- Misses         85       86       +1     
- Partials       35       36       +1     
Impacted Files Coverage Δ
mesa/visualization/ModularVisualization.py 77.92% <57.14%> (-0.89%) :arrow_down:

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 736367a...b10fe9f. Read the comment docs.

codecov[bot] avatar Apr 28 '22 11:04 codecov[bot]

I'm updating the CONTRIBUTING.rst to include this guide: https://github.com/projectmesa/mesa/pull/1298/commits/86b5095f09ae4dc2a954f97def0d9c4f863b1018. You need to run git config pull.rebase true in the Mesa Git repo, and the do a git pull from the Mesa upstream repo. With this, you won't see the messy merge commits. The workflow is that you will have to do git push --force (or git push -f) when pushing to GH remote, because you are rewriting history by rebasing your new commits on top of Mesa main, instead of doing merge.

rht avatar Apr 28 '22 13:04 rht

Shame on me, indeed passing the urlpath param makes no sense after ModularServer is instantiated. As it is not required I removed it. Also stripped slashes from URLPATH and added some doc. However, I struggle to run pre-commit as I have not python3.6 on my system. Why is language_version set to python3.6 in .pre-commit-config.yaml when mesa is built only for python >=3.7? Shouldn't python3 suffice? Further, in CONTRIBUTING.rst, shouldn't it read git commit -m "Fix X issue." instead of git commit -m "Fixes X issue." ;-)?

Holzhauer avatar May 02 '22 10:05 Holzhauer

Yeah, the .pre-commit-config.yaml (maybe just remove the language_version) and CONTRIBUTING.rst are definitely outdated. A PR to fix them would be appreciated!

rht avatar May 02 '22 10:05 rht

As it is not required I removed it. Also stripped slashes from URLPATH and added some doc.

I think these changes haven't been pushed to GH?

rht avatar May 03 '22 07:05 rht

Is there a reason why handlers are initialised outside init() in ModularServer? I wonder because this way it is hard to define a test which requires setting the environment var URLPATH before the initisalisation of handlers.

Holzhauer avatar Jul 27 '22 20:07 Holzhauer

Yeah, I think they are fine to be moved to __init__. But this refactor should be done in a separate PR, so that the PRs remain small and faster to merge.

rht avatar Jul 27 '22 23:07 rht

OK, could you then merge this PR before I prepare the refactor PR (otherwise there will be conflicts in ModularVisualization.py)?

Holzhauer avatar Jul 28 '22 08:07 Holzhauer

I do think the refactor should happen first. It is simpler to pass in urlpath as a Python argument than Bash env.

rht avatar Jul 28 '22 09:07 rht

If this is still actively followed please re-open the PR at https://github.com/projectmesa/mesa-viz-tornado But note that we are moving towards a new pure python frontend

Corvince avatar Aug 31 '23 20:08 Corvince