ivoa-std / DataLink

DataLink standard (DAL)
3 stars 6 forks source link

interface version attribute in example #96

Closed mbtaylor closed 10 months ago

mbtaylor commented 1 year ago

Section 2.2 contains an (optional) example capability element for a DataLink service, which starts like this:

   <capability standardID="ivo://ivoa.net/std/DataLink#links-1.0"
       xmlns:vs="http://www.ivoa.net/xml/VODataService/v1.1">
       <interface xsi:type="vs:ParamHTTP" role="std" version="1.0">
           ...

I'm not sure about the version attribute on the interface element here. Should it be 1.1 not 1.0 for this version of DataLink? I don't think this attribute suffers the same can-never-be-changed issues as the standardID value, though I might be wrong ... but in checking that out I read what VOResource says in its interface/@version documentation (VOResource 1.1 v3.2.2):

"The version of a standard interface specification that this interface complies with. Most VO standards indicate the version in the standardID attribute of the capability. For these standards, the version attribute should not be used."

so should that version attribute actually be removed?

msdemlei commented 1 year ago

On Thu, Feb 09, 2023 at 04:38:08AM -0800, Mark Taylor wrote:

Section 2.2 contains an (optional) example capability element for a DataLink service, which starts like this:

   <capability standardID="ivo://ivoa.net/std/DataLink#links-1.0"
       xmlns:vs="http://www.ivoa.net/xml/VODataService/v1.1">
       <interface xsi:type="vs:ParamHTTP" role="std" version="1.0">
           ...

I'm not sure about the version attribute on the interface element here. Should it be 1.1 not 1.0 for this version of DataLink? I don't think this attribute suffers the same can-never-be-changed issues as the standardID value, though I might be wrong ... but in checking that out I read what VOResource says in its @.***` documentation (VOResource 1.1 v3.2.2):

"The version of a standard interface specification that this interface complies with. Most VO standards indicate the version in the standardID attribute of the capability. For these standards, the version attribute should not be used."

so should that version attribute actually be removed?

Ah... Sigh. I think that's another topic we should work out in a common way in DALI, including getting clear why we'd like to indicate versions for which purposes in the first place.

Given that Datalink indicates the minor version in the standardID (which I'd argue now is a pattern we shouldn't follow, but since it's already there, I think we shouldn't touch it), it should not be using @version. So, I'd tend to agree with Mark: Let's drop @version in the example.

But given we've changed the table schema, we should now use

links-1.1 as the fragment identifier in the standardID.

mbtaylor commented 1 year ago

As far as I can tell, this issue hasn't been addressed in the recent PR-DataLink-1.1-20230413. The same thing comes up in section 3.3.1, which mandates an INFO in {links} responses like:

<INFO name="standardID" value="ivo://ivoa.net/std/DataLink#links-1.0"/>

So should that be changed to #links-1.1 as well? If so the .vor file needs a new <key> element too.

Putting the appropriate minor version in that standardID INFO is useful for validation, to indicate which DataLink version the {links} document claims to be implementing. However, I don't think that the validation tail should be wagging the specification dog, so if there's a good reason not to do that I don't insist.

msdemlei commented 1 year ago

On Mon, Apr 17, 2023 at 04:32:53AM -0700, Mark Taylor wrote:

Putting the appropriate minor version in that standardID INFO is useful for validation, to indicate which DataLink version the {links} document claims to be implementing. However, I don't think that the validation tail should be wagging the specification dog, so if there's a good reason not to do that I don't insist.

I don't think there's a good reason; there's probably not even a bad reason. I've just looked in pyVO's source, and it seems no code relies on the links-1.0 string. When it looks for datalink links services, it does a prefix search for ivo://ivoa.net/std/datalink#links and case-folds, as it should. So, that one's fine.

What's uglier is the #sync-1.0 that we're using for the descriptors of processing services. One could argue we don't change anything with them and keep the 1.0, but that would need to be explained somewhere then.

PyVO wrongly does a literal match for the sync id. I'll try to find a bit of time to fix this, but when we write #sync-1.1, that'll break current pyVO. Is this is a problem? No idea. I really don't know whether anyone is using that corner of pyVO -- there's not much docs on it, and not many examples either. I'd count that as a bad reason to keep it at best. The good reason would be that we didn't change the specs related to sync-1.0. Did we?

mbtaylor commented 1 year ago

I note that CADC are at time of writing using #links-1.1 in links response documents, contrary to the PR-DataLink-1.1-20230413 text:

% curl -s 'https://ws.cadc-ccda.hia-iha.nrc-cnrc.gc.ca/caom2ops/datalink?ID=ivo://cadc.nrc.ca/IRIS?f212h000/IRAS-25um' | grep 'DataLink#'
    <INFO name="standardID" value="ivo://ivoa.net/std/DataLink#links-1.1" />
pdowler commented 1 year ago

I think the #sync-1.0 mentioned above is actually SODA#sync-1.0 because SODA defines both sync and async. So aside from examples, this isn't directly applicable to DataLink spec and keys in a DataLink vor record.

Aside:

At CADC we implement both of those SODA endpoints (practically, we can do sync well and only add async as a convenience mechanism).

I recall that CASDA only implements SODA#async because of how they have to stage data (sometimes?) and cannot implement SODA#sync to their own satisfaction.

pdowler commented 1 year ago

As for the CADC datalink INFO output, that's probably something I had in some code and forgot to check back on this issue. I can easily revert to #links-1.0 since I guess we agreed we did not need to add a new key for optional columns... consider this a bug in the CADC service trying to prototype the new features.

I still don't think we are handling and using these keys properly. The whole point of versioned keys is that services can advertise that they support version-x.y features and clients can care about that or not by how they match against the standardID. By avoiding adding keys we make versions/features guess work. This isn't a DataLink specific issue and this isn't the place to argue about it... just don't know why we wouldn't add a new key.

msdemlei commented 1 year ago

On Mon, Apr 24, 2023 at 12:09:15PM -0700, Patrick Dowler wrote:

I think the #sync-1.0 mentioned above is actually SODA#sync-1.0 because SODA defines both sync and async. So aside from examples,

Yes, apologies. I should have checked the record before blurting out.

msdemlei commented 1 year ago

On Mon, Apr 24, 2023 at 12:20:25PM -0700, Patrick Dowler wrote:

As for the CADC datalink INFO output, that's probably something I had in some code and forgot to check back on this issue. I can easily revert to #links-1.0 since I guess we agreed we did not need

Well... if Mark says having #links-1.1 there makes his life easier, then that's reason enough for me to go for it unless we discover that breaks anything seriously that cannot easily be fixed. I, for one, have put in #links-1.1 into DaCHS yesterday and support fixing this identifier during RFC.

I still don't think we are handling and using these keys properly.

The trouble is that nobody ever properly spelled out what we want and don't want to accomplish with them, and how discovery, operation, and validation interact with them (yeah, the three have competing requirements). I've always wanted to write a note trying to work all that out, but then I never did.

DataLink specific issue and this isn't the place to argue about it... just don't know why we wouldn't add a new key.

Well, if components use the full ivoid to identify it's datalink (rather than a regular expression like ...#links-1.\d+), they'll unnecessarily break.

But that's probably an argument to introduce a new identifier now, because then these clients break early before they base even more functionalily on bad assumptions. Again, a note explaining how these identifiers should and should not be used would probably help.

pdowler commented 1 year ago

I do prefer to add #links-1.1 so services can say that's the standard they implemented. I agree we need to spell out somewhere how clients should use this information when found (either here in a links doc or in a service descriptor, which could these or another standardID).

If we agree (and maybe we did since multiple of is seem to think #links-1.1 is a thing), we should probably add it as a point on the RFC page and then I'll address it with a PR; maybe as "doc change that was lost"? :-)

Back when we had a single (initial value), it's hard to write the extra words to explain how to robustly deal with multiple values and motivate clients to do so when there is only one value (today). I would help write that note, but we should probably have a discussion at the interop about those 3 aspects.

jd-au commented 1 year ago

I recall that CASDA only implements SODA#async because of how they have to stage data (sometimes?) and cannot implement SODA#sync to their own satisfaction.

Yes that's correct - there are still times when we have to stage data. That will go away soon and we can consider if we can reliably offer a SODA#sync service.

mbtaylor commented 1 year ago

Right, I'd support raising this in RFC (I was planning to do it myself if nobody else does) and changing it to #links-1.1 for the REC. That addresses my issues with DataLink.

Do we additionally need to discuss how to do this in general? Should it be a topic for the TCG face to face meeting in Bologna?

mbtaylor commented 1 year ago

For completeness, this is being addressed by PR #109. For even more completeness, when digging into it some more just now I came across issue #62 from 2021 in which we discussed the same thing and came to the opposite conclusion. But I think we're doing the right thing now.