-
Notifications
You must be signed in to change notification settings - Fork 27
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
Enable public user #2
base: master
Are you sure you want to change the base?
Conversation
I've checked the IDR where we've decoupled server and web, and |
Yes, normally that is the case because web settings are only applicable to web, but as there is a server prerequisite explicitly for web in this case, it seemed to make sense to use the same setting key. |
@dpwrussell Could you rebase this please? @openmicroscopy/devops How does everyone feel about auto-creating a public user if |
I'm happy for public user config to be part "omero-server-docker 5.4" since that's currently recommended practice. It might be that with the true read-only work it'll look different in 5.5 or later, but so be it. |
Rebased and tested. |
Also tidy indentation and add comment
Note: currently looking at this. |
Enables the public user if the same settings that are required for the web are specified to the server.
I gave some thought to making this non-public-user specific, but this did not make much sense to me given its limited capabilities. I believe for that to be useful it would be necessary to build something much more complicated that takes configurations of users, groups and user-group memberships.
Even for this simple case, if it was generic it would not be logical to me (imagining myself an end user) that the server docker would also take one username and one groupname and that the given username would be in the given group.
No problem if you don't want to merge this in favour of a more powerful mechanism for adding users and groups, thanks to the modularised run scripts I can provide this in my package instead easily until that exists.
Depends on #1 I will rebase as that is merged.