S2439

[yeunga]

Added CadcAlmaResolver to support ALMA.

[brianmajor]

On Fri, Mar 29, 2019 at 9:46 AM yeunga notifications@github.com wrote:

@yeunga commented on this pull request.

In caom2-artifact-resolvers/src/main/java/ca/nrc/cadc/caom2/artifact/resolvers/CadcAlmaResolver.java https://github.com/opencadc/caom2/pull/101#discussion_r270492031:

+

• if (!StringUtil.hasText(fileID)) {
• throw new IllegalArgumentException("cannot extract fileID from " + uri);
• }
• sanitize(archive);
• sanitize(fileID);
• // trim the archive to 6 characters
• // TODO: Remove this trim when AD supports longer archive names
• if (archive.length() > 6) {
• archive = archive.substring(0, 6);
• }
• return archive + "/" + fileID;
• }

There is already a unit test for CadcAlmaResolver. I can add more asserts, e.g. check that there is no 'namespace' in the returned URL.

That won't work because the URL you'll receive may or may not have a namespace, depending on what the ad resolver does. Currently, the URL you get does have a 'namespace'. I think localizing the test to that method would be better and less fragile.

Brian

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/opencadc/caom2/pull/101#discussion_r270492031, or mute the thread https://github.com/notifications/unsubscribe-auth/AHk-AHaEltnMxaPc1XBUFe-3ZB0TfI40ks5vbkNdgaJpZM4cRFSu .

[brianmajor]

Okay.

On Fri, Mar 29, 2019 at 10:51 AM yeunga notifications@github.com wrote:

@yeunga commented on this pull request.

In caom2-artifact-resolvers/src/main/java/ca/nrc/cadc/caom2/artifact/resolvers/CadcAlmaResolver.java https://github.com/opencadc/caom2/pull/101#discussion_r270515932:

+

• if (!StringUtil.hasText(fileID)) {
• throw new IllegalArgumentException("cannot extract fileID from " + uri);
• }
• sanitize(archive);
• sanitize(fileID);
• // trim the archive to 6 characters
• // TODO: Remove this trim when AD supports longer archive names
• if (archive.length() > 6) {
• archive = archive.substring(0, 6);
• }
• return archive + "/" + fileID;
• }

In the unit test, if we set the artifact URI to "alma:ALMA/some/name/space/bar.fits", we can expect the returned URL to end with ALMA/bar.fits. This verifies that 'some/name/space/' has been removed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/opencadc/caom2/pull/101#discussion_r270515932, or mute the thread https://github.com/notifications/unsubscribe-auth/AHk-AHor-F5OuTqXhD9igaYMr3hH1r1wks5vblKVgaJpZM4cRFSu .