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

Allow for the override of SPECIFY_THICK_CLIENT from environment #5455

Open
wants to merge 5 commits into
base: production
Choose a base branch
from
Open

Allow for the override of SPECIFY_THICK_CLIENT from environment #5455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6dbe76c
Create test_sp7.xml
acwhite211 Dec 9, 2024
0004c44
Update specify_settings.py THICK_CLIENT_LOCATION override
acwhite211 Dec 9, 2024
bc8fc56
Update specify_settings.py
acwhite211 Dec 9, 2024
dd18c7a
Update test_sp7.xml
acwhite211 Dec 9, 2024
f2f30fa
Merge branch 'production' into test_sp7_static
acwhite211 Jan 24, 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
16 changes: 16 additions & 0 deletions config/test_sp7.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<!-- This file is to test that the sp7_only config files are beind used in the static files api -->
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this before merging, right?

<!-- Root element representing the main container -->
<test_sp7>
<!-- Comment explaining the purpose of this section -->
<section1>
<!-- Placeholder for a title -->
<title>SP7 Test</title>

<!-- Placeholder for a description -->
<description>SP7 Test</description>

<!-- Placeholder for a date -->
<date>YYYY-MM-DD</date>
</section1>
</test_sp7>
3 changes: 2 additions & 1 deletion specifyweb/settings/specify_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import os

# Specify 7 requires the files from a Specify 6 install.
# This setting should point to a directory containing an installation
# of Specify 6 of the same version as the Specify database.
THICK_CLIENT_LOCATION = '/opt/Specify'
THICK_CLIENT_LOCATION = os.environ.get('THICK_CLIENT_LOCATION', '/opt/Specify')
Copy link
Contributor

Choose a reason for hiding this comment

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

This allows local installations to set a THICK_CLIENT_LOCATION environment variable and use that instead of modifying this variable directly.

Do we want to add note of this to the installation instructions?

specify7/README.md

Lines 491 to 493 in 18f5c15

3. Make sure to update the `THICK_CLIENT_LOCATION` setting in
`local_specify_settings.py`, if you are updating the Specify 6
version.

Copy link
Contributor

Choose a reason for hiding this comment

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

And for Dockerized instances, do we want to provide documentation for how this environment variable can be used?
Or maybe just how it's used with correlation to the docker-compose.yml (and Dockerfile)?

For the users who wish to go in depth for customizing the setup of their instance, there's a LOT of moving parts. They might not even know what's possible/not-possible, and I could see how easy it might be to get stuck.

Copy link
Member

Choose a reason for hiding this comment

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

  1. I don't think we should encourage anyone to configure new local installations
  2. Documentation on supported environment variables are always desirable 😄


# Set the database name to the MySQL database you
# want to access which must be a Specify database already
Expand Down
Loading