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

Define omero.qa.feedback as new visible server property #38

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Define omero.qa.feedback as new visible server property #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 5 additions & 0 deletions src/main/resources/ome/config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
<property name="mutable" value="false"/>
<property name="visibility" value="all"/>
</bean>
<bean class="ome.system.Preference" id="omero.qa.feedback">
<property name="db" value="false"/>
<property name="mutable" value="false"/>
<property name="visibility" value="all"/>
</bean>
<bean class="ome.system.Preference" id="omero.data.dir">
<property name="mutable" value="true"/>
</bean>
Expand Down
6 changes: 6 additions & 0 deletions src/main/resources/omero-common.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ omero.security.trustStorePassword=
#############################################
omero.upgrades.url=http://upgrade.openmicroscopy.org.uk

#############################################
## QA configuration
#############################################
# Base URL to use when sending feedback (errors, comments)
omero.qa.feedback=http://qa.openmicroscopy.org.uk
Copy link
Member

Choose a reason for hiding this comment

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

I haven't read through the rest of the PR, but in terms of symmetry "omero.feedback.url" rings truer for me.

Copy link
Member

@jburel jburel Nov 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

we have omero.upgrades.url so omero.feedback.url or omero.qa.url will make sense

Copy link
Member Author

@sbesson sbesson Nov 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting thoughts. I went for the minimal amount of changes/extra configuration but happy to update to use a different property name if we reach a consensus.

After a bit of research, a few comments on QA vs feedback:

  • OMERO.qa is the historical name of the OME application which was built to receive the feedback from clients and handle upgrade checks - see https://omero.readthedocs.io/en/stable/users/history.html#beta-4-1-october-2009 for the original announcement
  • code-wise, the omero_qa namespace is primarily used internally within the app
  • the only relevant usage in the OMERO stack is the internal omero.qa.feedback OMERO.web property
  • OMERO.web has some omero.web.feedback.* settings allow to turn on/off part of the feedback mechanism
  • if we decided to go for omero.feedback.url, I suspect we would eventually try to deprecate omero.qa.feedback and update the client to consume omero.feedback.url

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for that summary, @sbesson. The fact that it's also documentation in sysadmin/config.rst leads me to say let's not change it. (At least in terms of the naming, I feel somewhat vindicated by the environment variable in https://github.com/ome/omero-web/blob/3ae446b01d98493b109998c1dece8224337db54a/omeroweb/settings.py#L267 😄)


#############################################
## cluster configuration
##
Expand Down