ivoa-std / DataLink

DataLink standard (DAL)
3 stars 6 forks source link

Data link #69 #69

Closed Bonnarel closed 2 years ago

Bonnarel commented 2 years ago

This PR is there in response to issue #52 and #55. Also creates CI preview.yml

Bonnarel commented 2 years ago

Le 25/10/2021 à 09:52, Mark Taylor a écrit :

@.**** requested changes on this pull request.


In DataLink.tex https://github.com/ivoa-std/DataLink/pull/69#discussion_r735340789:

@@ -835,6 +836,8 @@ \subsection{Service Resources} parse the URL to decide how to append additional parameters when invoking the service.

+In case the contentType is "text/html" the client SHOULD send the result of the service query to a web browser, would it be an html document or a web interactive interface.

I don't understand the wording here. Does "would it be" mean "this is appropriate for both HTML documents or web interactive interfaces"? Also, please try to keep line lengths short (<~80 characters), it makes diffs more comprehensible.


In DataLink.tex https://github.com/ivoa-std/DataLink/pull/69#discussion_r735341878:

The |exampleURL| is not explained anywhere, and (see Markus's comments at #55 https://github.com/ivoa-std/DataLink/issues/55) it's not immediately obvious what it is or how to use it. Please add some discussion in the text of how this should be filled in, and how clients are expected to make use of it.

OK. Will do it in the ADASS afternoon break !

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ivoa-std/DataLink/pull/69#pullrequestreview-787765449, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMP5LTBXOUZBQUY7WULZ6H3UIUEEJANCNFSM5GRKOCUQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Bonnarel commented 2 years ago

I made some changes in the PUIl request following Mark's recommendations. Note that this PR was based on the master which was valid until yesterday.

mbtaylor commented 2 years ago

I'm sorry Francois, I just don't understand what the text that you've added means. Also, I don't understand why "Clients SHOULD display exampleURL to the user" - why, and what's the user going to do with this? Perhaps somebody else who understands the intention of the exampleURL parameter could draft some alternative text or add some commentary to help me out?

Bonnarel commented 2 years ago

Le 26/10/2021 à 07:54, Mark Taylor a écrit :

I'm sorry Francois, I just don't understand what the text that you've added means.

Hi Mark. I imagined this is something similar to the /examples endpoint in VOSI. So mainly an help for client developers

Also, I don't understand why /"Clients SHOULD display exampleURL to the user"/ - why, and what's the user going to do with this?

I was probably confused by this comment/question raised by Markus "display them to the user?". Thinking to it a little bit more not sure it is so useful if it's mainly for the cleint developer

Perhaps somebody else who understands the intention of the exampleURL parameter could draft some alternative text or add some commentary to help me out?

I'il try to find a better text talking prievately to Roby.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ivoa-std/DataLink/pull/69#issuecomment-951584744, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMP5LTHZN5SVSV65NEGYO4DUIZGAXANCNFSM5GRKOCUQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

robyww commented 2 years ago

Here are three example URL use cases.

  1. When the service fails we would like to be able to show the user an error message that says something like

    “We are failing to call service xxx with url https://yyy.yyy.org/yyy. We might be constructing the url wrong. Here is an example from the provider - exampleURL”. So the user would only see the example URL in failure cases.

  2. There could be a info button about the service. We could s how the user documentation based on the service descriptor and an example URL.

  3. We are developing against the service and it is not working - we would like a URL to compare to. That way we can debug it to determine if we are doing something wrong or if we should contact the provider.

I don’t think we should say that clients should display the exampleURL to the user. However, there are uses cases where client might display it.

msdemlei commented 2 years ago

On Tue, Oct 26, 2021 at 07:30:09AM -0700, Trey Roby wrote:

Here are three example URL use cases.

  1. When the service fails we would like to be able to show the user an error message that says something like “We are failing to call service xxx with url https://yyy.yyy.org/yyy. We might be constructing the url wrong. Here is an example from the provider - exampleURL”. So the user would only see the example URL in failure cases.

Ok, so then the text around line 843 in the PR ought to be something like "Clients could give this example URL together with error messages as a debugging aid". That, I think, would be the minimal change.

  1. There could be a info button about the service. We could s how the user documentation based on the service descriptor and an example URL.

That is independent of having the example URL in the service descriptor, no?

  1. We are developing against the service and it is not working - we would like a URL to compare to. That way we can debug it to determine if we are doing something wrong or if we should contact the provider.

That's again the debugging use case.

I don’t think we should say that clients should display the exampleURL to the user. However, there are uses cases where client might display it.

Well, if we feel we need this, then I think we need to more clearly state where it came from to future implementors. Perhaps:

In exampleURL PARAMs, operators can give valid service calls as GET-able URLs in the PARAMs' value attributes. They are intended as an aid for debugging, in particular to aid users and developers in making sure a service is still operating as expected. The PARAM's description should give an indication of what the call will result in. End-user clients might indicate exampleURLs to the user after service failures.

I have to admit that even after writing this I'm not convinced this is actually a good way to address the problem, which I think is really close to what VOResource's testQueryString does (https://ivoa.net/documents/VOResource/20180625/REC-VOResource-1.1.html#tth_sEc3.2.2, "more on interfaces"). I give you that given the auxiliary nature of normal datalink services (that in general don't have capability endpoints of their own) makes it non-obvious how to sneak testQueryString-s into datalink workflows (which is another indication that we really need to tackle caproles).

Perhaps we can rename this testQueryString? I'd volunteer for writing a PR that would explain the rationale why this VOResource thing suddenly turns up in such an odd place.

robyww commented 2 years ago

That is independent of having the example URL in the service descriptor, no?

A better message could be given with an example URL.

There is nothing like something that we know works to understand a service. If we want to be able to auto-document a service we need a good example.

robyww commented 2 years ago

Perhaps we can rename this testQueryString? I'd volunteer for writing a PR that would explain the rationale why this VOResource thing suddenly turns up in such an odd place.

I don’t think it is odd. From a UI perspective if gives UI developers a significant tool.

The thing is that we want to make automatically make a UI application generate something just from a specification. If we had a known service if would be full of examples, generated descriptor information should be able to know and show the same thing.

msdemlei commented 2 years ago

On Tue, Oct 26, 2021 at 05:54:17PM -0700, Trey Roby wrote:

Perhaps we can rename this testQueryString? I'd volunteer for writing a PR that would explain the rationale why this VOResource thing suddenly turns up in such an odd place.

I don’t think it is odd. From a UI perspective if gives UI developers a significant tool.

Well, from a wider VO perspective it is a tad on the unusual side: if this weren't datalink (that normally doesn't have an endpoint from where to get capabilities yet), I'd definitely say: "use VOResource's testQueryString", as it's rarely a good thing to let people do the same thing in two different ways.

But we don't have capabilities for datalink services in general, so: fair enough.

But would you be fine with renaming exampleURL to testQueryString so we have a bit of consistency with VOResource at least in nomenclature?

The thing is that we want to make automatically make a UI application generate something just from a specification. If we had a known service if would be full of examples, generated descriptor information should be able to know and show the same thing.

Ah -- so, you're planning to generate forms for other operators' datalink services? If so, then I understand what you're saying in

issuecomment-952438664

Bonnarel commented 2 years ago

On Tue, Oct 26, 2021 at 07:30:09AM -0700, Trey Roby wrote: Here are three example URL use cases. 1. When the service fails we would like to be able to show the user an error message that says something like “We are failing to call service xxx with url https://yyy.yyy.org/yyy. We might be constructing the url wrong. Here is an example from the provider - exampleURL”. So the user would only see the example URL in failure cases. Ok, so then the text around line 843 in the PR ought to be something like "Clients could give this example URL together with error messages as a debugging aid". That, I think, would be the minimal change.

  1. There could be a info button about the service. We could s how the user documentation based on the service descriptor and an example URL. That is independent of having the example URL in the service descriptor, no?
  2. We are developing against the service and it is not working - we would like a URL to compare to. That way we can debug it to determine if we are doing something wrong or if we should contact the provider. That's again the debugging use case. I don’t think we should say that clients should display the exampleURL to the user. However, there are uses cases where client might display it. Well, if we feel we need this, then I think we need to more clearly state where it came from to future implementors. Perhaps: In exampleURL PARAMs, operators can give valid service calls as GET-able URLs in the PARAMs' value attributes. They are intended as an aid for debugging, in particular to aid users and developers in making sure a service is still operating as expected. The PARAM's description should give an indication of what the call will result in. End-user clients might indicate exampleURLs to the user after service failures.

+1 Personally I think it clarifies nicely what I understand from Trey's demand. If Trey is OK with text just above (to be written line 843) I prefer to modify my PR myself with that because there is some other stuff inside it.

I have to admit that even after writing this I'm not convinced this is actually a good way to address the problem, which I think is really close to what VOResource's testQueryString does (https://ivoa.net/documents/VOResource/20180625/REC-VOResource-1.1.html#tth_sEc3.2.2, "more on interfaces"). I give you that given the auxiliary nature of normal datalink services (that in general don't have capability endpoints of their own)

Humm. It is true that {links} endpoints may not be registered and my not have VOSI endpoints. But service descriptors have not been defined for {links} endpoints or {links} services although they are in the same doc, although they also can do it (example 4.1). They can be in any kind of VOTAble response. Some people even wanted to push that to a next VOTable and splitting them from the core DataLink They are generally providing descriptions of other services (such as SODA, SIA, etc....) and even were intended to describe non standard services in order to help building UI. That's why this exampleURL is duplicating nothing in general, I think. After saying that, renaming exampleURL into testQueryString, I have nothing against, but exampleURL is clear and nice too and recall the /examples endpoint in DALI. So other people : thoughts about the name ?

makes it non-obvious how to sneak testQueryString-s into datalink workflows (which is another indication that we really need to tackle caproles). Perhaps we can rename this testQueryString? I'd volunteer for writing a PR that would explain the rationale why this VOResource thing suddenly turns up in such an odd place.

According what I wrote above, I don't think we need that. Your text above is nice enough.

robyww commented 2 years ago

In exampleURL PARAMs, operators can give valid service calls as GET-able URLs in the PARAMs' value attributes. They are intended as an aid for debugging, in particular to aid users and developers in making sure a service is still operating as expected. The PARAM's description should give an indication of what the call will result in. End-user clients might indicate exampleURLs to the user after service failures.

I think this is good.

Bonnarel commented 2 years ago

In exampleURL PARAMs, operators can give valid service calls as GET-able URLs in the PARAMs' value attributes. They are intended as an aid for debugging, in particular to aid users and developers in making sure a service is still operating as expected. The PARAM's description should give an indication of what the call will result in. End-user clients might indicate exampleURLs to the user after service failures.

I think this is good.

After this long discussion I replaced text written yesterday by text proposed by Markus and approved by Trey. should be the end of it !

Bonnarel commented 2 years ago

OK Pat, if you are merging. This was not done from your last master but from the one just before (still valid on sunday/monday)

Le 28/10/2021 à 14:45, Mark Taylor a écrit :

@.**** approved this pull request.

The content of this text looks OK to me now (though it could use some line breaks).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ivoa-std/DataLink/pull/69#pullrequestreview-791813382, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMP5LTEKB6C2UEU6XD64KSTUJFHZNANCNFSM5GRKOCUQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jd-au commented 2 years ago

I've resolved the conflicts and ensured the PARAMs are closed in the example

pdowler commented 2 years ago

All: this question from Markus wasn't addressed:

But would you be fine with renaming exampleURL to testQueryString so we have a bit of consistency with VOResource at least in nomenclature?

Mark seems OK with exampleURL and I'm on the fence... there is also the option of a sibling DALI-examples document (sibling of the endpoint in the service descriptor's accessURL value) that could offer both explanatory text and example(s) that work. Is that especially onerous for client writers? This is the 3rd way to convey a working example in the VO...

Bonnarel commented 2 years ago

Hi all, Le 03/11/2021 à 02:56, Patrick Dowler a écrit :

All: this question from Markus wasn't addressed:

But would you be fine with renaming exampleURL to testQueryString so
we have a bit of consistency with VOResource at least in nomenclature?

Mark seems OK with exampleURL and I'm on the fence...

well exampleURL is pretty simple, but I'm not opposing testQueryString id preferred

there is also the option of a sibling DALI-examples document (sibling of the endpoint in the service descriptor's accessURL value) that could offer both explanatory text and example(s) that work. Is that especially onerous for client writers? This is the 3rd way to convey a working example in the VO...

Service descriptors are not fully redondant with capability, DALI examples and all that sort of things for two reasons

     1 ) they are valid also for non standard services. Any VOTable response from whatever service may contain a service descriptor to describy any parameter based URL. In non standard case that's the only way to describe the interface

    2 ) even in the case of standard services they may be context dependent (an there are in genral otherwise capability could have been sufficient) . The simplest dependence for SODA, DataLink itself, etc... is to work with a predefined ID= .... But there others (OPTIONS eg)

Si I'm not sure it's that important to refer to testQyeryString or DALI examples here

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ivoa-std/DataLink/pull/69#issuecomment-958595093, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMP5LTBDBN2NXYOPCBGAFR3UKCQHRANCNFSM5GRKOCUQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mbtaylor commented 2 years ago

I'm not massively against any of the options for the detail of how to provide an example URL (exampleURL/testQueryString/DALI-examples). DALI-examples makes some kind of sense and allows more flexibility and explanatory content. However from a service provider's point of view, implementing it is quite a bit more effort than sticking another PARAM in alongside the existing ones, and my guess is that we'd get many fewer services providing an /examples endpoint (which some might support) than adding an example URL in a PARAM (which we could reasonably expect everybody to do). Also, a casual reader of the service descriptor RESOURCE who's less familiar with the standards is more likely to stumble across the PARAM than go looking for an /examples document. The name of the PARAM doesn't really matter, testQueryString could be used as a concession to standardisation. So I'd say that's my preferred option.

pdowler commented 2 years ago

If we are going to nod toward VOResource (w/ testQuery) or DALI-examples (w/ exampleURL) I guess I'd prefer the latter, especially with respect to the templated URL issue and RFC6570.