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

no file or directory mandatory #558

Merged
merged 8 commits into from
Sep 28, 2022

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Aug 18, 2022

This PR should fix #459

The config file is now not mandatory, but the server will not start if one of the variables $QUETZ_SQLALCHEMY_DATABASE_URL and $QUETZ_SESSION_SECRET is missing.
The deployment directory is not mandatory either if the store is not a LocalStore. In fact if any other available store has a variable defined.

This also adds some basic tests to ensure server does not start without minimal settings.

MISSING TESTS : make sure the server starts correctly without settings file but with environment variables. Haven't found a way to kill it cleanly, so all following tests will fail with Address already in use.

@brichet brichet added the enhancement New feature or request label Aug 18, 2022
@brichet
Copy link
Collaborator Author

brichet commented Sep 16, 2022

ad6c7e6 allows to create a local storage without configuration file.

WARNING : If the database is sqlite, the database url includes the local path to the database file. This database URL can be set from the configuration file or the environmental variable, and currently we don't know where it comes from.
In that case, if a configuration file exists, the database path will be relative to that config file path. If the configuration file does not exist, the database path will be relative to the current working directory.

This means that if the database url is set in environmental variable and is relative to CWD and a configuration file exists in an other directory => it will fail to start.

@brichet brichet force-pushed the fix/no_files_directories_mandatory branch from ad6c7e6 to 79c676a Compare September 19, 2022 07:44
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2022

Codecov Report

Base: 82.92% // Head: 82.92% // No change to project coverage 👍

Coverage data is based on head (1467892) compared to base (1467892).
Patch has no changes to coverable lines.

❗ Current head 1467892 differs from pull request most recent head 79c676a. Consider uploading reports for the commit 79c676a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #558   +/-   ##
=======================================
  Coverage   82.92%   82.92%           
=======================================
  Files          78       78           
  Lines        5987     5987           
=======================================
  Hits         4965     4965           
  Misses       1022     1022           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@hbcarlos hbcarlos left a comment

Choose a reason for hiding this comment

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

Thanks, @brichet! I left two minor comments.
Everything else looks good to me.

quetz/cli.py Show resolved Hide resolved
quetz/testing/fixtures.py Outdated Show resolved Hide resolved
@hbcarlos hbcarlos merged commit 5d83914 into mamba-org:main Sep 28, 2022
@brichet brichet deleted the fix/no_files_directories_mandatory branch September 28, 2022 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployment directory shouldn't be mandatory
3 participants