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

asmeurer · 2023-08-30T05:59:45Z

Still some issues that need to be worked out here. Also will need #549 to be merged and rebased first.

This work is part of #513.

Description

This pull request:

a
b
c

Pull request checklist

Did you test this change locally?
Did you update the documentaion (if required)?
Did you add/update relevant tests for this change (if required)?

Additional information

costrouc · 2023-08-30T15:30:19Z

Much saner default considering before how it would just write to your current directory. Agree this is needed.

asmeurer · 2023-08-30T20:38:23Z

Also need to double check that there aren't any places that were relying on the default values without configuring it (e.g., in the Docker setup).

There is currently an issue where it is not able to access the database file correctly, which I am still figuring out.

netlify · 2023-09-15T23:00:43Z

✅ Deploy Preview for kaleidoscopic-dango-0cf31d ready!

Name	Link
🔨 Latest commit	`08cc13b`
🔍 Latest deploy log	https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/6528696958777b00087c99b5
😎 Deploy Preview	https://deploy-preview-554--kaleidoscopic-dango-0cf31d.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

asmeurer · 2023-09-15T23:01:53Z

Hmm, previously I was having issues where the database file was not being initialized properly, but now I'm not. I'm not sure if something else changed in the interim that changed this, or if I'm just not doing the same steps I was before. It would be helpful if someone else can test this (just run conda-store-server --standalone, or alternatively run conda-store-server and conda-store-worker separately, and see if it appears to function and that there aren't any errors printed to the console).

trallard · 2023-09-26T16:24:20Z

@nkaretnikov to test

nkaretnikov · 2023-10-01T17:49:19Z

conda-store-server/conda_store_server/app.py

@@ -200,7 +201,7 @@ class CondaStore(LoggingConfigurable):
    )

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


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.

Tested on Linux: works without changes.

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

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.

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

See for instance 80d9ea8

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.

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

nkaretnikov · 2023-10-01T17:49:53Z

conda-store-server/conda_store_server/app.py

@@ -87,7 +88,7 @@ class CondaStore(LoggingConfigurable):
    )

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


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

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.

nkaretnikov · 2023-10-01T17:58:06Z

conda-store-server/conda_store_server/server/app.py

@@ -179,6 +179,8 @@ def initialize(self, *args, **kwargs):

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

+        self.conda_store.ensure_directories()


Note for other reviewers: this runs:

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

because store_directory might not exist.

nkaretnikov · 2023-10-01T18:00:24Z

conda-store-server/tests/conftest.py

@@ -24,9 +24,13 @@ def conda_store_config(tmp_path, request):

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


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

asmeurer · 2023-10-04T22:10:57Z

Adding another note here: I think it would be a good idea to rename ~/.conda-store/conda-store-state to just ~/.conda-store/state, which is not only cleaner and less redundant, but should help with #588 as well.

And use PurePosixPath instead of PosixPath, because PosixPath is not present on Windows.

asmeurer · 2023-10-06T21:36:51Z

Tested this on Windows by manually merged it with #559

asmeurer · 2023-10-12T21:48:19Z

I've addressed the review comments.

One thing worth being aware of here is that if anyone is running conda-store without configuring database_url or store_directory but just relying on the defaults, their setup will break. All the example configs do configure it, so hopefully that isn't happening.

nkaretnikov · 2023-10-13T12:36:13Z

One thing worth being aware of here is that if anyone is running conda-store without configuring database_url or store_directory but just relying on the defaults, their setup will break. All the example configs do configure it, so hopefully that isn't happening.

Wait, I think it wasn't an issue before, no? Why is this required now? Like, I'm pretty sure I used to be able to just run with --standalone without passing the config option.

asmeurer · 2023-10-13T15:06:24Z

The point is it will break existing installations if they were relying on the default configuration, because the defaults have changed.

nkaretnikov · 2023-10-13T15:10:35Z

Ah, OK. How about at least printing an error message and suggesting users move stuff to a new location?
However, since the file names are also different now, it might still be confusing if we just provide written instructions.
Ideally, this would be done automatically if we're confident we don't overwrite user data in home or something like that.

asmeurer · 2023-10-13T15:15:15Z

Hmm, the test failure is because tests conda_store object from the fixture isn't being used for some reason.

nkaretnikov · 2023-10-18T14:43:01Z

@asmeurer The test failure looks legit. You also need to rebase.

________________ test_conda_store_register_environment_workflow ________________

...

ERROR    celery.app.trace:trace.py:270 Task task_update_storage_metrics[43b3f586-1076-4ec6-8185-6b17c6e0bda6] raised unexpected: FileNotFoundError(2, 'No such file or directory')
Traceback (most recent call last):
  File "/usr/share/miniconda/envs/conda-store-server-dev/lib/python3.10/site-packages/celery/app/trace.py", line 477, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/home/runner/work/conda-store/conda-store/conda-store-server/conda_store_server/worker/tasks.py", line 75, in task_update_storage_metrics
    conda_store.configuration(db).update_storage_metrics(
  File "/home/runner/work/conda-store/conda-store/conda-store-server/conda_store_server/orm.py", line 663, in update_storage_metrics
    disk_usage = shutil.disk_usage(store_directory)
  File "/usr/share/miniconda/envs/conda-store-server-dev/lib/python3.10/shutil.py", line 1280, in disk_usage
    st = os.statvfs(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/.conda-store/state'

nkaretnikov · 2023-10-21T14:44:31Z

Taking over this one. See #639.

pavithraes added the status: in progress 🏗 label Aug 30, 2023

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

fbe6145

There is currently an issue where it is not able to access the database file correctly, which I am still figuring out.

asmeurer force-pushed the standalone-state-home-dir branch from 0ade4c6 to fbe6145 Compare August 30, 2023 21:13

Set the store_directory to a temp directory in the tests

ac5e648

asmeurer mentioned this pull request Sep 15, 2023

Windows support #559

Closed

3 tasks

Merge branch 'main' into standalone-state-home-dir

a8d3524

kcpevey requested a review from pierrotsmnrd September 19, 2023 16:09

asmeurer mentioned this pull request Sep 20, 2023

Figure out how to handle long paths on Windows #588

Closed

trallard assigned asmeurer Sep 26, 2023

nkaretnikov reviewed Oct 1, 2023

View reviewed changes

asmeurer added 4 commits October 5, 2023 14:23

Merge branch 'main' into standalone-state-home-dir

753c49d

Factor out CONDA_STORE_DIR

cd927e0

And use PurePosixPath instead of PosixPath, because PosixPath is not present on Windows.

Fix typo

861b76d

Fix sqlite database path

6b1c97e

asmeurer added 2 commits October 12, 2023 15:28

Log the location of the database and storage directories

066f646

Make the tests conda-store dirs similar to the standalone defaults

004e533

asmeurer marked this pull request as ready for review October 12, 2023 21:46

asmeurer changed the title ~~[WIP] Store the state and database files in ~/.conda-store by default~~ Store the state and database files in ~/.conda-store by default Oct 12, 2023

Run black

08cc13b

trallard added the needs: review 👀 label Oct 17, 2023

asmeurer mentioned this pull request Oct 17, 2023

Revisit the implementation of --standalone as well as the assumptions made #513

Closed

nkaretnikov mentioned this pull request Oct 21, 2023

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

Merged

nkaretnikov closed this Oct 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

asmeurer commented Aug 30, 2023

costrouc commented Aug 30, 2023

asmeurer commented Aug 30, 2023

netlify bot commented Sep 15, 2023 •

edited

Loading

asmeurer commented Sep 15, 2023

trallard commented Sep 26, 2023

nkaretnikov Oct 1, 2023

nkaretnikov Oct 1, 2023

nkaretnikov Oct 1, 2023

asmeurer Oct 2, 2023 •

edited

Loading

asmeurer Oct 2, 2023

nkaretnikov Oct 3, 2023

asmeurer Oct 5, 2023

nkaretnikov Oct 1, 2023

nkaretnikov Oct 1, 2023

nkaretnikov Oct 1, 2023

nkaretnikov Oct 1, 2023

asmeurer commented Oct 4, 2023

asmeurer commented Oct 6, 2023

asmeurer commented Oct 12, 2023

nkaretnikov commented Oct 13, 2023

asmeurer commented Oct 13, 2023

nkaretnikov commented Oct 13, 2023

asmeurer commented Oct 13, 2023

nkaretnikov commented Oct 18, 2023

nkaretnikov commented Oct 21, 2023

		@@ -179,6 +179,8 @@ def initialize(self, args, *kwargs):

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

		self.conda_store.ensure_directories()

		@@ -24,9 +24,13 @@ def conda_store_config(tmp_path, request):

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

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

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

Conversation

asmeurer commented Aug 30, 2023

Description

Pull request checklist

Additional information

costrouc commented Aug 30, 2023

asmeurer commented Aug 30, 2023

netlify bot commented Sep 15, 2023 • edited Loading

✅ Deploy Preview for kaleidoscopic-dango-0cf31d ready!

asmeurer commented Sep 15, 2023

trallard commented Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmeurer Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmeurer commented Oct 4, 2023

asmeurer commented Oct 6, 2023

asmeurer commented Oct 12, 2023

nkaretnikov commented Oct 13, 2023

asmeurer commented Oct 13, 2023

nkaretnikov commented Oct 13, 2023

asmeurer commented Oct 13, 2023

nkaretnikov commented Oct 18, 2023

nkaretnikov commented Oct 21, 2023

netlify bot commented Sep 15, 2023 •

edited

Loading

asmeurer Oct 2, 2023 •

edited

Loading