-
Notifications
You must be signed in to change notification settings - Fork 501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IQSS/7177 enhance metrics api #7178
IQSS/7177 enhance metrics api #7178
Conversation
This is up-to-date w.r.t. 5.4 including the merge and updating the flyway script. I found an extraneous bit of text in the guide file that was causing the build error. I'm not sure if that also fixes the text size issue. If not, it's beyond my .rst knowledge to adjust the size - if anyone can point to an example of how to structure such a list, I'd be happy to apply it. W.r.t. to backward compatibility - I just added a 6 line PR in the dataverse-metrics repo that adds the now required Accept:application/json header that would be needed to keep the an existing metrics UI deployment working with Dataverse instances with the PR here. There's also the larger PR over in dataverse-metrics that creates all the new graphs that this PR supports from the Dataverse side which includes the same fix. Depending on what gets merged there (and into what release) by the time this PR is released, the release notes could say something like requiring an update to 0.2.8 to continue retrieving global metrics and updating to 0.3.0 to enable the new per Dataverse collection metrics UI for the local installation. |
@qqmyers you're probably in the best position to add a release note. If you could do that it would be most appreciated. |
Yeah, me too. I just pushed a "good enough" fix in 1e4a4cf after getting the go ahead from @qqmyers . To make the text size reasonable, I converted the sub items to a bulleted list as in the "Total" example below. Longer term we should probably try to figure out what's going on so people who are trying to write docs like this don't need bullets. I looked at https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#indentation but couldn't get anything better working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't run the code myself but this seems to be ready for QA.
As of Dataverse 5.5, specifically IQSS/dataverse#7178 , an "Accept" header is required to download metrics in JSON format (the default is CSV). This was affecting the retrieval of subject.
What this PR does / why we need it: Enhances and standardizes the design of metrics api endpoints
Which issue(s) this PR closes:
Closes #7177
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here: no
Is there a release notes update needed for this change?:
Additional documentation: