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

Use OS specific application directories instead of ~/omero #242

Merged
merged 10 commits into from Sep 14, 2020

Conversation

manics
Copy link
Member

@manics manics commented Aug 11, 2020

This is #233 rebased and modified to only use the user_cache_dir property. get_omero_userdir() is unchanged (still returns ~/omero) but this can be changed to user_data_dir in 6.0.0 when breaking changes are allowed.

You can enable the new behaviour for get_omero_userdir() by setting OMERO_USERDIR="" (empty string, as opposed to not setting it).

Related to #210 but does not fully resolve it- need to wait until the new behaviour is the default

No migration options are supported since the cache dir has not yet been released. Longer term I'm inclined not to, AFAIK at the moment sessions are the only meaningful state stored under ~/omero, and since a major release probably implies a major release of the server and therefore a restart there's no point migrating an old session.

Decisions:

  • Include the major version (currently 5, will be 6 in future) in the directory path or not.

Revert potentially breaking changes to get_omero_userdir

Partial revert of 0f0d44e

You can enable appdirs for get_omero_userdir by setting OMERO_USERDIR="" as opposed to unset
@jburel
Copy link
Member

jburel commented Aug 19, 2020

if the user wants to test the new feature, anything stores in the current selected location will be not be moved since there is no migration is offered, maybe the doc should indicate that.
Insight is in maintenance mode, but we should probably unify that cc @dominikl

@manics
Copy link
Member Author

manics commented Aug 20, 2020

Done.

A bit of googling found https://github.com/harawata/appdirs

@jburel
Copy link
Member

jburel commented Aug 21, 2020

insight issue created ome/omero-insight#160

Thanks for the change.

@manics manics mentioned this pull request Aug 27, 2020
3 tasks
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

One question for everyone re: OMERO.cli. Otherwise, seems straight-forward enough:

bash-4.2$ .venv3/bin/python -m "omero.util.temp_files" dir
/home/omero/omero/tmp/omero_omero/2432
bash-4.2$ OMERO_USERDIR="" .venv3/bin/python -m "omero.util.temp_files" dir
/home/omero/.local/share/OMERO.cli/5/tmp/omero_omero/2434

src/omero/util/__init__.py Outdated Show resolved Hide resolved
test/unit/test_util.py Outdated Show resolved Hide resolved
test/unit/test_util.py Outdated Show resolved Hide resolved
test/unit/test_util.py Outdated Show resolved Hide resolved
Co-authored-by: Sébastien Besson <seb.besson@gmail.com>
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

👍 can certainly live with the use of "OMERO.py" for the moment which will at least cover the napari use case. I don't know of anyone who has made use of those files from Java.

@sbesson sbesson merged commit 02f4bad into ome:master Sep 14, 2020
@manics manics deleted the appdirs branch September 14, 2020 10:51
@ehrenfeu
Copy link

Hi all, especially @manics and @sbesson

I'd like to point out that (if I understood correctly) with the changes planned for 6.0.0 in get_omero_userdir() it will be difficult to e.g. run an omero import if the system user doesn't have write permissions to its $HOME directory.

At least I haven't found a way to influence appdirs choice on the path, e.g. through an environment variable.

This might sound esoteric, but it's affecting me in hrm-omero as the way the code usually gets called on an HRM installation is through the system user running the web server (e.g. www-data) and quite often (e.g. on CentOS 7) that one is not allowed to write to /var/www/ or similar. In that case the download and extraction of OMERO.java.zip fails (silently) and the import process breaks.

Would it be possible to have an optional way to influence where that stuff ends up in the file system?

Cheers,
Niko

@ehrenfeu
Copy link

ehrenfeu commented Oct 15, 2021

Me again...

Maybe I misunderstood the intention / plan of @manics - in case the idea is to retain the possibility of influencing the cache path by setting OMERO_USERDIR then everything is fine. What I'd like to avoid is having to monkeypatch the HOME environment variable as this is guaranteed to have a rat-tail of unforeseen side-effects (I have some history of shooting myself in the foot with runtime-patching code...).

❤️
Niko

@will-moore
Copy link
Member

I don't see any other changes planned for 6.0.0 that corresponds to what's in the docstring In 6.0.0 the default will change to use appdirs.user_data_dir, so maybe we should open a PR to do that?

But even in that case, you can still override by setting OMERO_USERDIR.

@ehrenfeu
Copy link

Thanks @will-moore - if setting OMERO_USERDIR will continue to work I am all fine.

It's just that I read the "In 6.0.0 the default will change to use appdirs.user_data_dir" that the appdirs.user_data_dir will have priority over the OMERO_USERDIR variable. That's what got me worried.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants