-
-
Notifications
You must be signed in to change notification settings - Fork 810
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
feat(playlists): POST endpoint for importing m3u playlists - #2078 #2273
Conversation
Download the artifacts for this pull request: |
Thanks @SwatsonCodes! I'll review as soon as I can (the backlog of PRs to review is huge ATM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. See my comments below. Also, we could add some tests here, specially to the ImportM3U
method
Hey @spwats, are you planning in addressing these changes? If not, would it be ok if I work on them? |
Hi @caiocotts! Yes please, I'd be happy for you to pick up this PR. I feel bad for abandoning it, but I got a new job and no longer have time to work on open source stuff. |
It's all good man. Thanks for the fast response! |
Hey @deluan, I made the changes you requested, but I still have to write the tests. Let me know if there's anything else I'm missing. |
Looks good, thanks! Let me know if you need help with the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tests! Just a small change and we are ready to merge!
Thanks @spwats and @caiocotts! |
Very happy to see this get completed! Thanks for picking up where I left off @caiocotts. |
I tried this out this morning using the develop docker image. I get the following response when I try to do an import:
A playlist is created with a generic name based on timestamp and the test song is not added. I do not see a way to set the playlist name. Thinking it should come from the playlist filename??? Here is my script I am testing:
Here is what the playlist I am testing with looks like. I have copied the line from another playlist that was exported so I know the path is correct.
The token is properly obtained. I am able to export playlists so I think I have all of the URL's correct. Guessing I am missing something, but not quite sure what. |
Hey @samcro1967. As for the song not getting added. Is the path As a reference, this is the M3U I was using to test the endpoint, my navidrome was running directly on my machine without docker:
|
Yes, /music/ is what is mounted to the docker container. I have validated that the path is correct.
I get the same results with the following playlist.m3u:
|
Does this endpoint require EXTM3U formatted lists? If so, which EXTM3U lines exactly are required to be contained in the source m3u files? |
I was able to get the import to work with the updated script below.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Closes 2078
Description:
adds an HTTP post endpoint at
api/playlist
for uploading m3u playlistsChanges:
api/playlist
based on content-type. Requests withaudio-xmpegurl
content type are parsed as a m3u playlist and imported.io.Reader
. Previous code assumed a local file.Example POST request: