IRSA-7208: Support additional MOCs #1862
Conversation
|
it works for me! although i note that the moc overlay is really, really, really slow. like, almost longer than my ADHD timescale. |
robyww
force-pushed
the
IRSA-7208-MOCs-available
branch
from
d6216e7 to
ed702de
Compare
There was a problem hiding this comment.
Looks good! Tested the MOCs, and the additional bug tickets as well, but have some comments about them below:
Firefly-1863: I couldn't recreate this bug on ops/dev, but it seems to be working here.
IRSA-7339 and Firefly-1881: Could you build a ZTF and Spherex to test these two?
Approving the PR with some other minor comments, would be nice to test ZTF and Spherex (I was able to recreate those bugs on ops/dev).
| StringBuilder out= new StringBuilder(); | ||
| int cnt=0; | ||
| for(var a : r.getSendHeaders().entrySet()) { | ||
| var k = a.getKey();; |
There was a problem hiding this comment.
small fix, accidental double semi colon here.
| if (r.isReservedKey(k) && !k.equalsIgnoreCase("<none>") && | ||
| !k.equalsIgnoreCase("content-type") && !k.equalsIgnoreCase("content-length") && | ||
| !k.equalsIgnoreCase("Last-Modified") ) { |
There was a problem hiding this comment.
I spent some time studying this...should it be
if ( !r.isReservedKey(k) ... followed by the other conditions, instead?
There was a problem hiding this comment.
you right! but the name is right the function is reversed.
| } catch (URISyntaxException | MalformedURLException ignore) { | ||
| return null; | ||
| } | ||
| return Util.Try.it(() -> new URI(urlStr).toURL()).getOrElse((URL)null); |
There was a problem hiding this comment.
I think we the trim and null check made sense (at least the trim, I understand the Try util will take care of the null exception. So suggested change :
if (urlStr == null) return null;
return Util.Try.it(() -> new URI(urlStr.trim()).toURL()).getOrElse((URL)null);
You can skip the first line, but might make sense to keep the .trim() in there.
There was a problem hiding this comment.
FYI: get() returns null by default if there's an exception. .getOrElse() is used when you want a different default value.
| sources: getDefaultMOCList(), | ||
| label: 'Featured MOC ' | ||
| }, | ||
| adhocMocIncludeAdditionSources: 'irsa', |
There was a problem hiding this comment.
minor text suggestion:
this could be changed to adhocMocIncludeAdditionalSources everywhere.
- includes reponse to feedback More work; - fixes Firefly-1870 - fixes Firefly-1863 - fixes IRSA-7339 - fixes Firefly-1881 - fixes test failing
robyww
force-pushed
the
IRSA-7208-MOCs-available
branch
from
13bcc02 to
143274a
Compare
4 participants
IRSA-7208: Support additional MOCs
Other Tickets
Review
Testing
HiPS SearchpanelHiPS / MOC->Add MOC Layeributtons in the props columns, these are the additional MOCsEuclid planned data