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

Store the state and database files in ~/.conda-store by default #554

Closed
wants to merge 10 commits into from
Closed

Store the state and database files in ~/.conda-store by default #554

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fbe6145
Store the state and database files in ~/.conda-store by default
asmeurer Aug 30, 2023
ac5e648
Set the store_directory to a temp directory in the tests
asmeurer Aug 30, 2023
a8d3524
Merge branch 'main' into standalone-state-home-dir
asmeurer Sep 15, 2023
753c49d
Merge branch 'main' into standalone-state-home-dir
asmeurer Oct 5, 2023
cd927e0
Factor out CONDA_STORE_DIR
asmeurer Oct 5, 2023
861b76d
Fix typo
asmeurer Oct 6, 2023
6b1c97e
Fix sqlite database path
asmeurer Oct 6, 2023
066f646
Log the location of the database and storage directories
asmeurer Oct 12, 2023
004e533
Make the tests conda-store dirs similar to the standalone defaults
asmeurer Oct 12, 2023
08cc13b
Run black
asmeurer Oct 12, 2023
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
5 changes: 3 additions & 2 deletions conda-store-server/conda_store_server/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import datetime
import os
import sys
from pathlib import Path, PosixPath
from typing import Any, Dict

import pydantic
Expand Down Expand Up @@ -87,7 +88,7 @@ class CondaStore(LoggingConfigurable):
)

store_directory = Unicode(
"conda-store-state",
str(Path.home() / ".conda-store" / "conda-store-state"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a variable for Path.home() / ".conda-store" and use it everywhere. For example: CONDA_STORE_DIR.

Copy link
Contributor

Choose a reason for hiding this comment

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

The location of CONDA_STORE_DIR needs to be printed when the server is started.

Ideally: you can even print something like "Found existing conda-store.sqlite in <dirname>" or "Creating conda-store.sqlite in <dirname>". Same for the rest of top-level state files/dirs.

Otherwise, we'll run into problems when reproducing issues. People will have no idea that some state was created somewhere. Up until now, it hasn't been a problem because files are more visible in the current project dir, since it's tracked by git.

help="directory for conda-store to build environments and store state",
config=True,
)
Expand Down Expand Up @@ -200,7 +201,7 @@ class CondaStore(LoggingConfigurable):
)

database_url = Unicode(
"sqlite:///conda-store.sqlite",
"sqlite:///" + str(PosixPath.home()) + "/.conda-store/conda-store.sqlite",
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to remove Posix, it doesn't work on Windows:

-        "sqlite:///" + str(PosixPath.home()) + "/.conda-store/conda-store.sqlite",
+        "sqlite:///" + str(Path.home()) + "/.conda-store/conda-store.sqlite",

Tested on Windows: after rebasing on #559 and fixing the Posix thing, the server started (python -m conda_store_server.server) and stored state files in the home dir.
Screen Shot 2023-10-01 at 19 42 39

Copy link
Contributor

Choose a reason for hiding this comment

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

Tested on Linux: works without changes.

% ls -1
conda-store.sqlite
conda-store-state
% pwd
/home/nkaretnikov/.conda-store

Copy link
Contributor

Choose a reason for hiding this comment

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

Verified that this env builds on Linux and is available after restart (with the same state dir):

channels:
- conda-forge
dependencies:
- python
- pip:
  - nothing
- ipykernel
- pytest
- requests
description: ''
name: test-env
prefix: null
variables: null

Server was started via python -m conda_store_server.server --standalone.

Copy link
Contributor Author

@asmeurer asmeurer Oct 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Are you sure about that? PosixPath is used for URLs because URLs always use forward slashes, even on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See for instance 80d9ea8

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about that? PosixPath is used for URLs because URLs always use forward slashes, even on Windows.

Have you tested on Windows? When I ran it on Windows, it raised an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently on Windows you have to use pathlib.PurePosixPath instead of pathlib.PosixPath

help="url for the database. e.g. 'sqlite:///conda-store.sqlite' tables will be automatically created if they do not exist",
config=True,
)
Expand Down
2 changes: 2 additions & 0 deletions conda-store-server/conda_store_server/server/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ def initialize(self, *args, **kwargs):

self.conda_store = CondaStore(parent=self, log=self.log)

self.conda_store.ensure_directories()
Copy link
Contributor

Choose a reason for hiding this comment

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

Note for other reviewers: this runs:

os.makedirs(self.store_directory, exist_ok=True)

because store_directory might not exist.


if self.conda_store.upgrade_db:
dbutil.upgrade(self.conda_store.database_url)

Expand Down
4 changes: 4 additions & 0 deletions conda-store-server/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ def conda_store_config(tmp_path, request):

filename = pathlib.Path(tmp_path) / "database.sqlite"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not essential, but needs ".conda-store" here, to be consistent with how we run this outside of tests.


store_directory = tmp_path / ".conda-store" / "conda-store-state"
store_directory.mkdir(parents=True)

with utils.chdir(tmp_path):
yield Config(
CondaStore=dict(
store_directory=str(store_directory),
database_url=f"sqlite:///{filename}?check_same_thread=False"
)
)
Expand Down