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

Create a configuration (admin) page for various variables #50

Open
7 tasks
joj0s opened this issue Jun 30, 2020 · 7 comments
Open
7 tasks

Create a configuration (admin) page for various variables #50

joj0s opened this issue Jun 30, 2020 · 7 comments
Assignees
Labels
Priority: High Should be prioritised over other issues Scope: Backend Backend logic & data processing scripts
Milestone

Comments

@joj0s
Copy link
Collaborator

joj0s commented Jun 30, 2020

There are parameters which we would like to be able to tweak, such as BASE_URLs for external datasources. Creating a config file to read from would provide a easy solution to that.

  • Base URLs for datasources
    • ZOOMA
    • OLS
    • OxO
    • ClinVar
  • OxO mapping distance (default: 2)
  • Number of reviews necessary to consider the trait reviewed (default: 2)
@joj0s joj0s added the Scope: Backend Backend logic & data processing scripts label Jun 30, 2020
@tskir
Copy link
Member

tskir commented Aug 1, 2020

I think this is better implemented as a configuration page, which will only be available to admins and will list all parameters and allows their modification just by editing the fields.

This “admin” page can actually be the same one with what is described in #32

@tskir tskir changed the title Create a config file to hold the app's configuration variables Create a configuration (admin) page for various variables Aug 1, 2020
@tskir
Copy link
Member

tskir commented Aug 1, 2020

I will add the things needed to be configurable through this interface to the issue description as a checklist.

@joj0s
Copy link
Collaborator Author

joj0s commented Aug 20, 2020

So besides creating a config page to tweak all those parameters, we still have to solve the problem of hardcoded configuration variables and secrets. The way I see it, the options are either to use env variables, or an external config file. Which way do you prefer to follow?

@tcezard
Copy link
Member

tcezard commented Aug 20, 2020

My personal preference is a yaml file. It is easy to read and an example can be made in the repository to make it easier to configure. Some people also use config in python directly but that might not be appropriate here.

@joj0s
Copy link
Collaborator Author

joj0s commented Aug 23, 2020

@tcezard I can confirm we can have config variables that can be edited via the Django Admin and the changes are persistent. Here is the package to do this https://pypi.org/project/django-admin-conf-vars/, I have tested it and it works great (we are going to have to use a regular python file instead of a yaml file though.

@joj0s
Copy link
Collaborator Author

joj0s commented Aug 23, 2020

Update: There is actually a problem with this approach. We can't use this package in the settings.py file because the settings are loaded before the app has been initialized.

Now, the good thing is that the only settings that we would want to read from an external file in the settings.py file are the SECRET_KEY setting and the database credentials. So I think we should separate the credentials and secret_key part of the settings in another file, and use the 'admin_config_vars` package for the variables that alter the app's functionality (basically every variable that has been defined for this issue). After all, the credentials shouldn't be editable by anybody in the first place. This would also allow us to commit the app's configuration file to this repo.

@tskir tskir added the Priority: High Should be prioritised over other issues label Aug 27, 2020
@tskir tskir added this to the 1.0 milestone Aug 27, 2020
@tskir
Copy link
Member

tskir commented Aug 27, 2020

I've read the discussions above and agree with the approach: having some low-level settings hardcoded in settings.py is fine, and all the rest we'll configure with admin_config_vars package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Should be prioritised over other issues Scope: Backend Backend logic & data processing scripts
Projects
None yet
Development

No branches or pull requests

3 participants