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

extend CLI admin email timeout #5608

Merged
merged 2 commits into from
Jan 12, 2018
Merged

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Jan 5, 2018

What this PR does

Sending e-mail to a user of our demo server takes around 15s. This PR extends the timeout for bin/omero admin email ... so as to be more patient with such slow servers.

Testing this PR

Use the CLI to send e-mail to a demo server user and check that the e-mail is received and omero::LockTimeout is not thrown at you. (I am mtbcarroll on that server and I am happy to make anyone else an account on it.) Can also try out the --wait option.

Related reading

https://trello.com/c/bSyOy8Bh/34-cli-or-email-plugin-5s-timeout

@joshmoore
Copy link
Member

@mtbc
Copy link
Member Author

mtbc commented Jan 5, 2018

Ah, so close this PR and add --wait facility to the email subcommand?

@joshmoore
Copy link
Member

Keeping this as a bump of the default timeout makes sense, too.

@mtbc mtbc force-pushed the extend-email-timeout branch from b29e43f to 63595a5 Compare January 8, 2018 09:17
@dominikl
Copy link
Member

Looks good, works like expected. Good to merge 👍

@mtbc mtbc added this to the 5.4.2 milestone Jan 11, 2018
@joshmoore
Copy link
Member

@kennethgillen : If there's anywhere in particular you think this should be advertised, please let us know.

@joshmoore joshmoore merged commit 622b27d into ome:next_infra Jan 12, 2018
@mtbc mtbc deleted the extend-email-timeout branch January 15, 2018 08:06
@kennethgillen
Copy link
Member

It should probably be an example somewhere on https://docs.openmicroscopy.org/latest/omero/sysadmins/index.html

I missed this myself. Might have been worth a "sysadmin update" in the release notes.

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.

4 participants