Skip to content
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

Merged

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Aug 11, 2020

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:

@qqmyers
Copy link
Member Author

qqmyers commented Apr 7, 2021

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.

@pdurbin
Copy link
Member

pdurbin commented Apr 8, 2021

@qqmyers you're probably in the best position to add a release note. If you could do that it would be most appreciated.

@pdurbin pdurbin self-assigned this May 10, 2021
@pdurbin
Copy link
Member

pdurbin commented May 10, 2021

it's beyond my .rst knowledge to adjust the size

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.

Screen Shot 2021-05-10 at 11 27 31 AM

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.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ May 10, 2021
Copy link
Member

@pdurbin pdurbin left a 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.

@kcondon kcondon merged commit 64c3142 into IQSS:develop May 12, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 May 12, 2021
@djbrooke djbrooke added this to the 5.5 milestone May 12, 2021
pdurbin added a commit to IQSS/dataverse.harvard.edu that referenced this pull request Jun 24, 2021
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Metrics API enhancement
5 participants