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

Http redirection opt #1791

Closed
wants to merge 35 commits into from
Closed

Http redirection opt #1791

wants to merge 35 commits into from

Conversation

@lukh
Copy link
Contributor

lukh commented Aug 23, 2019

Hi all ! first, I want to thank you for this amazing project ! i have learned a lot working around it..
I want to propose a simple way to redirect web request from the domain to a specific web app, via the config file...
exemple: localhost:6680 redirects to localhost:6680/mopidy by default
a
root_redirection = whatever
in http section of the config file will redirect localhost:6680 to localhost:6680/whatever

it allows to set a default landing page (iris), avoiding the clients.html page.

@lukh lukh closed this Aug 29, 2019
@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Aug 29, 2019

sorry, didn't merge with the right branch. reopening it with develop

@lukh lukh reopened this Aug 29, 2019
@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Aug 29, 2019

I did remerge with current dev branch of mopidy

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 15, 2019

Codecov Report

Merging #1791 into develop will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1791      +/-   ##
===========================================
+ Coverage    82.17%   82.19%   +0.01%     
===========================================
  Files           77       77              
  Lines         6318     6324       +6     
===========================================
+ Hits          5192     5198       +6     
  Misses        1126     1126
Impacted Files Coverage Δ
mopidy/http/actor.py 63.33% <100%> (+1.92%) ⬆️

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 ff991b6...8c6213b. Read the comment docs.

lukh added 2 commits Sep 15, 2019
@kingosticks

This comment has been minimized.

Copy link
Member

kingosticks commented Sep 15, 2019

This is a good idea and since it is asked quite often we might as well do it. But do you think we can have a more user-friendly name? Maybe default_path or default_webclient?

@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Sep 16, 2019

I wasn't very happy with root_redirection neither,

Copy link
Member

kingosticks left a comment

This would be even nicer with a couple of tests.

mopidy/http/actor.py Outdated Show resolved Hide resolved
mopidy/http/ext.conf Outdated Show resolved Hide resolved
@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Sep 25, 2019

here we are !

mopidy/http/actor.py Outdated Show resolved Hide resolved
mopidy/http/actor.py Outdated Show resolved Hide resolved
tests/http/test_server.py Outdated Show resolved Hide resolved
tests/http/test_server.py Outdated Show resolved Hide resolved
@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Nov 28, 2019

Hello,
I don't really know what are the next steps.
Do you plan to accept the merge ? What is the normal way to do ? (I am not used to open source developpment as a contributor, please excuse me if I don't do things as it should be...)

mopidy/http/actor.py Outdated Show resolved Hide resolved
mopidy/http/actor.py Outdated Show resolved Hide resolved
@kingosticks

This comment has been minimized.

Copy link
Member

kingosticks commented Nov 29, 2019

You've done a good job, thanks for all your efforts. I've been busy lately but I can take a proper look again on the weekend and hopefully get this in.

@lukh

This comment has been minimized.

Copy link
Contributor Author

lukh commented Nov 29, 2019

Thank you !

@jodal
jodal approved these changes Nov 29, 2019
Copy link
Member

jodal left a comment

I like this. Let's get it merged this weekend :-)

tests/http/test_server.py Outdated Show resolved Hide resolved
docs/ext/http.rst Outdated Show resolved Hide resolved
mopidy/http/actor.py Outdated Show resolved Hide resolved
mopidy/http/actor.py Outdated Show resolved Hide resolved
tests/http/test_server.py Outdated Show resolved Hide resolved
tests/http/test_server.py Outdated Show resolved Hide resolved
@jodal

This comment has been minimized.

Copy link
Member

jodal commented Nov 30, 2019

I've looked through and marked previous converstations as resolved, so I think the current list of open converstations is all that is left for us to be able to merge this. Nice work :-)

@jodal jodal added this to the v3.0 milestone Nov 30, 2019
@jodal
jodal approved these changes Dec 1, 2019
tests/http/test_server.py Outdated Show resolved Hide resolved
docs/changelog.rst Outdated Show resolved Hide resolved
@jodal jodal closed this in c90ffc4 Dec 2, 2019
@jodal

This comment has been minimized.

Copy link
Member

jodal commented Dec 2, 2019

Thanks for your patience :-)

@kingosticks

This comment has been minimized.

Copy link
Member

kingosticks commented Dec 2, 2019

Awesome, thanks @lukh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.