draft: Updated TableWriter to include service descriptors in query results#209
draft: Updated TableWriter to include service descriptors in query results#209aratikakadiya wants to merge 7 commits into
Conversation
| String filename = fid + ".xml"; | ||
| if (serviceDocument == null) { | ||
| return; | ||
| return; // TODO: verify - continue/return? |
There was a problem hiding this comment.
Please confirm if we have to return if valid document is not found for one of the fieldId.
There was a problem hiding this comment.
I don't recall the origin of that check but it should not terminate (current usage is likely why we never noticed). Probably something like this is better:
if (serviceDocument != null) {
for (VOTableResource metaResource : serviceDocument.getResources()) {
...
pdowler
left a comment
pdowler
left a comment
There was a problem hiding this comment.
some minor tweaks needed, otherwise looks right.
Have you considered some intTest(s) in youcat?
- create table with IDs (2-3)
- add single-ID and multi-ID service descriptor templates
- execute queries that select different column combinations
Would not need any rows in the table to get the VOTable result and verify the right descriptors are there.
| } | ||
|
|
||
| protected void populateAccessURLParam(VOTableResource metaResource) throws MalformedURLException { | ||
| RegistryClient regClient = new RegistryClient(); |
There was a problem hiding this comment.
I would probably make regClient a member var rather than recreate each time this method is called. To be conservative, private transient ... and instantiate it here if it is null (lazxy init).
| String filename = fid + ".xml"; | ||
| if (serviceDocument == null) { | ||
| return; | ||
| return; // TODO: verify - continue/return? |
There was a problem hiding this comment.
I don't recall the origin of that check but it should not terminate (current usage is likely why we never noticed). Probably something like this is better:
if (serviceDocument != null) {
for (VOTableResource metaResource : serviceDocument.getResources()) {
...
| } catch (URISyntaxException e) { | ||
| throw new RuntimeException("resourceIdentifier in " + filename + " is invalid", e); | ||
| } | ||
| populateAccessURLParam(metaResource); |
There was a problem hiding this comment.
it doesn't matter technically, but probably populateAccessURLParam before add as you have in the youcat-specific extension
2 participants
No description provided.