Support additional Exporter MIME Types #10295
Conversation
update from develp
|
@lubitchv - have you been able to look at using the new external exporter mechanism for this? It would be nice to have a real exporter use it and that would allow v6.2 (v6.1?) instances to drop it in, but since you created this ~in parallel, I don't think we should hold this up if you'd rather this go forward as is (or if we need to see if anyone else is interested/able to make the changes to package it as an external exporter jar). |
|
No, I did not yet looked at the external exporters. If you could push this PR as it is it would be great. I do plan to look at the external exporters, just did not have time to do that. |
|
The current plan is to use the separate PR at gdcc/dataverse-exporters#11 to produce an external jar for the ddi pdf exporter and to modify this PR to just include changes/docs required in the core repository. Changes TBD so this is waiting for changes before continuing through review/QA. |
|
@qqmyers is that still the plan? To use https://github.com/gdcc/dataverse-exporters ? I'm asking because I prefer a separate repo, like I did for https://github.com/gdcc/exporter-croissant Either way, this PR needs to be converted to "docs only", right? Should this be moved back to "in progress"? |
There was a problem hiding this comment.
Code will go to https://github.com/gdcc/exporter-ddipdf. This PR will just be the api change.
|
|
||
| @Override | ||
| public String getFormatName() { | ||
| return "pdf"; |
There was a problem hiding this comment.
Should probably be more unique than this - perhaps ddi-pdf or codebookpdf? (I see the HTML Codebook exporter is just 'html' but with external exporters allowed, it would be better to avoid accidental name conflicts.
There was a problem hiding this comment.
I think it has to correspond to the name in the api call for exporters and in UI page call for the exporter, in other words it is a name of the exporter that is used in calls of the exporter
|
|
||
| @Override | ||
| public String getMediaType() { | ||
| return MediaType.WILDCARD; |
There was a problem hiding this comment.
Can this be pdf? I see that Datasets adds / but does this have to match that to work?
There was a problem hiding this comment.
I tried pdf, it does not seem to work and the only thing that worked was WILDCARD
| @@ -0,0 +1,5 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <xsl:stylesheet version="2.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:outline="http://worldbank.org/toolkit/cdrom/outline" exclude-result-prefixes="outline"> | |||
| <xsl:param name="language-code" select="'en'"/> | |||
There was a problem hiding this comment.
Does this mean it is always using en? Or is there some way that the language code is getting sent in?
There was a problem hiding this comment.
Yes, it is always using English in this case. I do not know how to sent to xsl, that is why I were asking you about the language.
| @@ -206,7 +206,7 @@ public Response getDataset(@Context ContainerRequestContext crc, @PathParam("id" | |||
| // WORKS on published datasets, which are open to the world. -- L.A. 4.5 | |||
| @GET | |||
| @Path("/export") | |||
| @Produces({"application/xml", "application/json", "application/html", "application/ld+json" }) | |||
| @Produces({"application/xml", "application/json", "application/html", "*/*" }) | |||
There was a problem hiding this comment.
@lubitchv - Is this the only line that needs to get into core Dataverse at this point? And if so, can it leave the application/ld_json and add / after that? (Then all the current internal exporters are covered explicitly - a minor point.)
Beyond that, #10533 is adding a place to document new exporters in the guides, so it would be nice to add the new ddi-pdf exporter there, but, if this line is needed to make it work and we want that in v.6.3, we 'll need a one line PR asap and the docs could get updated afterwards.
BTW - just created a gdcc/exporter-ddipdf and am asking Oliver to help get the jar out to maven central. I'll add you to it.
There was a problem hiding this comment.
Yes, I think "/" can be added after "application/ld+json"
There was a problem hiding this comment.
@lubitchv - great - if you can make this change and delete the rest of the changes, this can go forward.
There was a problem hiding this comment.
Coordinating w/ @lubitchv and @poikilotherm, the ddipdf previewer is now in https://github.com/gdcc/exporter-ddipdf and this PR is just a one line change to add "." to the list of possible mimetypes exporters can produce, which is a general improvement to support external exporters.
For QA, probably regression testing is all that's needed, although on could check the open api output (assuming that includes allowed mimetpyes).
qqmyers
changed the title
What this PR does / why we need it:
Per comments, this PR which originally added a PDF formatted DDI codebook exporter has been reduced to just adding "." as a possible response type in the exporter API. The exporter itself now follows the external exporter model and is in the gdcc/exporter-ddipdf repo.
Which issue(s) this PR closes:
Closes #9481
Special notes for your reviewer:
Suggestions on how to test this:
See comments at the end.
Does this PR introduce a user interface change? If mockups are available, please link/include them here: