Open
Conversation
In VOResource 1.1, we added testQuery to vr:Interface; hence, the more specialised testQueryString is no longer necessary.
trjaffe
reviewed
Sep 26, 2025
| query URL is formed by the concatenation of the | ||
| base URL (given by the accessURL) and the value | ||
| given by this testQuery element. | ||
| This is a legacy element; use testQueryString from |
There was a problem hiding this comment.
Am I confused or is this backward? I thought you were deprecating testQueryString.
There was a problem hiding this comment.
I agree with Tess, this is weird: apparently the code change shows you are removing testQuery whereas the commit is about removing testQueryString. BTW, we are using testQuery for validation purposes, so please do not deprecate it !
msdemlei
changed the title
Collaborator
Author
|
On Fri, Sep 26, 2025 at 11:30:55AM -0700, Renaud Savalle wrote:
I agree with Tess, this is weird: apparently the code change shows
you are removing testQuery whereas the commit is about removing
testQuery**String**. BTW, we are using testQuery for validation
purposes, so please do not deprecate it !
So, the plan has indeed been to deprecate testQuery, which is in
vs:ParamHTTP, and tell people to migrate to testQueryString, which is
directly from vr:Interface (since VOResource 1.1). The hope would be
that people migrate to testQueryString over time.
What I'm trying to fix is that vs:ParamHTTP now has two children that
do the same thing. I wish I had noticed this earlier: the dangers of
inheritance, when I added testQueryString in VOResource I quite
certainly had not noticed how weird this would look in
VODataService's vs:ParamHTTP.
Anyway: We have to do *something* to explain the situation. I am
still convinced that testQueryString on vr:Interface is the right
thing, because that way any derivation from interface gets to define
some sort of request payload for test purposes, and that seems a good
thing to me. Just saying "vs:ParamHTTP's testQuery is what everyone
uses, so forget testQueryString for this particular type" is an
option, too, but not one I'd particularly enjoy.
Apologies for having written the PR title and description in exactly
the wrong way for *this* particular proposal. I've just fixed them.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In VOResource 1.1, we added testQueryString to vr:Interface; hence, the more specialised testQuery from vs:ParamHTTP is no longer necessary.