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

Extend LOVE manager routing system for subpath app serving #196

Merged
merged 10 commits into from
Jul 18, 2023

Conversation

sebastian-aranda
Copy link
Contributor

This PR refactors the routing system of the LOVE-manager in order to allow serving the app from a sub path (e.g. /love).

@sebastian-aranda sebastian-aranda changed the title Extend LOVE-manager routing system Extend LOVE-manager routing system for subpath app serving Jul 17, 2023
@sebastian-aranda sebastian-aranda changed the title Extend LOVE-manager routing system for subpath app serving Extend LOVE manager routing system for subpath app serving Jul 17, 2023
Copy link
Contributor

@mareuter mareuter left a comment

Choose a reason for hiding this comment

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

Minor comment and a suggestion, but otherwise this looks good.

README.md Outdated
@@ -40,6 +40,7 @@ All these variables are initialized with default variables defined in :code:`.en
- `LOVE_SITE`: defines the site name of the LOVE system. This value is used to identify the LOVE system in the LOVE-manager.
- `REMOTE_STORAGE`: defines if remote storage is used. If this variable is defined, then the LOVE-manager will connect to the LFA to upload files. If not defined, then the LOVE-manager will store the files locally.
- `COMMANDING_PERMISSION_TYPE`: defines the type of permission to use for commanding. Currently two options are available: `user` and `location`. If `user` is used, then requests from users with `api.command.execute_command` permission are allowed. If `location` is used, then only requests from the configured location of control will be allowed. If not defined, then `user` will be used.
- `URL_SUBPATH`: defines the path where the LOVE-manager will be served. If not defined, then requests will be served from the root path `/`. Note: the application has its own routing system, so this variable must be thought as a prefix to the application's routes.
Copy link
Contributor

Choose a reason for hiding this comment

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

thought as a to though of as a

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thought of as as right?

Copy link
Contributor

Choose a reason for hiding this comment

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

thought of as a prefix ...

@@ -17,6 +17,10 @@
# Quick-start development settings - unsuitable for production
# See https://docs.djangoproject.com/en/2.2/howto/deployment/checklist/

# Define the URL subpath for this application when deployed:
FORCE_SCRIPT_NAME = os.environ.get("URL_SUBPATH")
Copy link
Contributor

Choose a reason for hiding this comment

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

You could set the default value here as "" so that the if FORCE_SCRIPT_NAME checks become unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to do that in a beginning, but then I realized this is a Django setting variable, thus it is thought to be None if no string is defined. I did't try this out, but I think if set "" as default value, some things could stop working.

@sebastian-aranda sebastian-aranda merged commit 995ae19 into develop Jul 18, 2023
@sebastian-aranda sebastian-aranda deleted the tickets/DM-39986 branch July 18, 2023 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants