uwescience / myria-web

Web frontend for Myria
https://demo.myria.cs.washington.edu
Other
11 stars 14 forks source link

calendar warning only for rest.myria.cs... #242

Closed radion closed 9 years ago

radion commented 9 years ago

Should the "Myria cluster is reserved" message show up if you're not connected to rest.myria.cs.washington.edu?

dhalperi commented 9 years ago

please update from master for the fix I put in #243

dhalperi commented 9 years ago

Also note that we want the calendar warning present if the deployment uses any machines in the cluster. The reservation is for the hardware, not the rest.myria.cs.washington.edu deployment.

Not sure how to make that happen; this check is not sufficient.

domoritz commented 9 years ago

Do other deployments usually use the rest.myria.cs.washington.edu url or an ip?

dhalperi commented 9 years ago

.. or vega.cs.washington.edu, or dbserver02.cs.washington.edu, or ...

domoritz commented 9 years ago

Then let's just use a regex for those. I think that if somebody has their own deployment they may submit their queries manually anyway. I think the main use of the calendar is on demo.myria.cs.washington.edu anyway.

dhalperi commented 9 years ago

sure -- if the server name is localhost we can skip the check. Anything else, I'm happy making the developer comment out the line of JS.

This is unrelated to manual queries -- proposed change could only affect users that are using the website, anyway...


Daniel Halperin Director of Research for Scalable Data Analytics eScience Institute University of Washington

On Mon, Jan 5, 2015 at 12:50 PM, Dominik Moritz notifications@github.com wrote:

Then let's just use a regex for those. I think that if somebody has their own deployment they may submit their queries manually anyway. I think the main use of the calendar is on demo.myria.cs.washington.edu anyway.

— Reply to this email directly or view it on GitHub https://github.com/uwescience/myria-web/pull/242#issuecomment-68778813.

radion commented 9 years ago

So do we want to only filter out just localhost, regex the list of machines, use rest.myria..., or leave the code as it was? I'll update the code with the changes we want here.

dhalperi commented 9 years ago

Here's my calculus:

false positive: fraudulent warning, and to disable warning, developer has to figure out the right JS and disable

false negative: warning where there should be one. cluster reservations may be inadvertently violated through myria-web.

To me, cost of false negative is MUCH higher, false positive is minor inconvenience at most

I feel like the last one is optimal, but would hear other opinions.

radion commented 9 years ago

I agree, for now, we should go with the localhost option. That's the main reason I thought of this change.

The ideal solution, I think, is to create a set of reserved worker machines that the code checks against. But I don't know if that's a possibility.