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.

Sign up for GitHub

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

Details for different port/address added #3124

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+17 −4
Open

Details for different port/address added #3124

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b3779c9
Update docs and config for user working on different port/address
granth23 Jan 13, 2025
d8943bf
Deployment docs remove unwanted changes
granth23 Jan 13, 2025
18ec98b
Docs updated
granth23 Jan 13, 2025
4d3eb90
Config Updated
granth23 Jan 13, 2025
e57ef05
Merge branch 'config-docs-update' of https://github.com/granth23/list…
granth23 Jan 13, 2025
c35f733
Merge branch 'metabrainz:master' into config-docs-update
granth23 Jan 13, 2025
bd0e6a5
Update devel-env.rst
granth23 Jan 13, 2025
0e1f62c
Config and Docs Updated
granth23 Jan 15, 2025
42be7ab
Unwanted Changes
granth23 Jan 15, 2025
8be1996
Merge branch 'master' into config-docs-update
granth23 Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/developers/devel-env.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,16 @@ configuration.
cp listenbrainz/config.py.sample listenbrainz/config.py
Now, open the new config.py file (don’t change config.py.sample) with your favorite
text editor and look for this section.
text editor.

.. note::

If you are accessing your development server using a port other than ``8100``,
ensure that you update the ``SERVER_ROOT_URL`` field to reflect the appropriate port number.
Copy link
Member

Choose a reason for hiding this comment

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

There are potentially quite a few more configuration lines that would need to be modified, so I wonder if a more generic advice such as "search for 8100 in the config file and replace it with the port you want to use" or something along those lines would be more suited and more future-proof.

For example, API_URL, SPOTIFY_CALLBACK_URL and other callback URLs use the port number.

Alternatively, you could modify the sample config again to replace localhost:8100 with SERVER_ROOT_URL in an f-string like we do for MUSICBRAINZ_BASE_URL :

listenbrainz-server/listenbrainz/config.py.sample

Line 85 in 82e3eb6

OAUTH_AUTHORIZE_URL = f"{MUSICBRAINZ_BASE_URL}/new-oauth2/authorize"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the necessary changes by using a f-string format.

If you are accessing your development server using a host other than ``localhost`` (e.g., GitHub Codespaces),
ensure that you update the ``SERVER_NAME`` and ``SERVER_ROOT_URL`` fields to reflect the appropriate host details.

Next look for this section in the file.

.. code-block:: yaml
Expand Down
5 changes: 4 additions & 1 deletion listenbrainz/config.py.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ SQLALCHEMY_TIMESCALE_URI = "postgresql://listenbrainz_ts:listenbrainz_ts@lb_db/l
TIMESCALE_ADMIN_URI = "postgresql://postgres:postgres@lb_db/postgres"
TIMESCALE_ADMIN_LB_URI = "postgresql://postgres:postgres@lb_db/listenbrainz_ts"

# Server address
SERVER_NAME = "localhost:8100"
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why setting this SERVER_NAME is required?
I only see it use in the testing framework so i am wondering.

As mentioned in my previous comment, this could potentially use the value of SERVER_ROOT_URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In situations where generating absolute external URLs is required, such as during OAuth redirection, Flask relies on the SERVER_NAME configuration. Without setting SERVER_NAME, Flask defaults to localhost, which can lead to incorrect or invalid redirect URLs. To ensure OAuth and similar features point to the correct local or production URL, it is necessary to configure SERVER_NAME with the actual base URL of the application.

While SERVER_ROOT_URL serves as a full external-facing URL, it is not directly usable for Flask’s purposes since it includes the protocol (http:// or https://), which Flask does not interpret in SERVER_NAME. Therefore, the two values, while related, cannot be interchanged directly.

You can refer to the attached Flask Documentation.


# MetaBrainz - used for retrieving donation info from metabrainz.org
SQLALCHEMY_METABRAINZ_URI = ""

Expand Down Expand Up @@ -211,4 +214,4 @@ REJECT_LISTENS_WITHOUT_USER_EMAIL = False
REJECT_NEW_USERS_WITHOUT_EMAIL = False

# base directory for user data exports
USER_DATA_EXPORT_BASE_DIR = "/code/listenbrainz/exports/"
USER_DATA_EXPORT_BASE_DIR = "/code/listenbrainz/exports/"
Copy link
Member

Choose a reason for hiding this comment

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

Newline deleted automatically? You might need to adjust your code editor settings to not remove them automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will look into it.

Loading