Skip to content

Dm 11387 Add ability to limit what will appears as background jobs #431

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

Merged
merged 7 commits into from
Aug 3, 2017

Conversation

loitly
Copy link
Contributor

@loitly loitly commented Aug 1, 2017

https://jira.lsstcorp.org/browse/DM-11387

Background jobs from one page(client app) may not be applicable in another page. This PR add the ability to limit what appears as background jobs.

Use /firefly/firefly.html and /firefly/firefly-dev.html to test that they do share background searches.
User /irsaviewer/ and /irsaviewer/timeseries to test that they are setup to not share background jobs.

loitly added 4 commits August 1, 2017 11:31
- on fail search, mask panel remained without error messages.
- API should have backgroundable set to false, since background monitor may not be available.
@loitly loitly requested review from ejoliet and robyww August 1, 2017 20:51
Copy link
Contributor

@ejoliet ejoliet left a comment

Choose a reason for hiding this comment

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

Couple of things i've noticed that you might want to check (and fix) before merging:

  • Email notification doesn't reset, keep repopulating the email in the input field.
  • Prepare for download dialog keeps shown even after the job is done.

I've checked the rest and it looks good.

loitly added 2 commits August 1, 2017 15:58
- added a checkbox to enable email notification
- sync the checkbox and email field between download dialog and background monitor
- remove 'resend email' feature which is confusing
- when email notification is enabled, an email will be sent for each job when it's completed.
@loitly
Copy link
Contributor Author

loitly commented Aug 2, 2017

I've push a new commit which fixes several issues related to backgrounding.

  • closes the download dialog immediately after submit. logic for when to close was fuzzy.
  • added a checkbox to enable email notification
  • sync the checkbox and email field between download dialog and background monitor
  • remove 'resend email' feature which is confusing
  • when email notification is enabled, an email will be sent for each job when it's completed.

@ejoliet , please review again to see if the PR is ready to merge.

Copy link
Contributor

@ejoliet ejoliet left a comment

Choose a reason for hiding this comment

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

Changes are great! The email enabling feature make more sense now although i didn't receive an email but it might be my local deployment.
Good to go.

@ejoliet
Copy link
Contributor

ejoliet commented Aug 2, 2017

email notification works. I was on wireless network, once i switch to LAN, i got the email. SMTP trick... so all good.

@loitly loitly merged commit 2517643 into dev Aug 3, 2017
@loitly loitly deleted the DM-11387_filter_bgJobs_from_app branch August 3, 2017 16:12
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.

2 participants