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.

Sign up for GitHub

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

Jump to bottom

Counter metrics table #6124

Merged
merged 34 commits into from
Aug 28, 2023
Merged

Counter metrics table #6124

merged 34 commits into from
Aug 28, 2023

Conversation

orangewolf
Copy link
Member

@orangewolf orangewolf commented Jul 25, 2023 •
edited by summer-cook
Loading

Related

Summary

Adds a table for counter metrics to hyrax for PALs implementation

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  • Run the spec associated with this PR and make sure it passes.

Changes proposed in this pull request:

Passing test showing the table properties are valid ![Uploading image.png…]()

Notes

  • We will likely have to make changes to this table(/create more tables?) depending on what other metrics are needed

@samvera/hyrax-code-reviewers

@summer-cook summer-cook changed the title Counter i hardly knew her Counter metrics table Jul 26, 2023
@summer-cook summer-cook marked this pull request as ready for review July 26, 2023 16:42
@summer-cook summer-cook mentioned this pull request Jul 27, 2023
Closed
2 tasks
alishaevn
alishaevn previously approved these changes Jul 27, 2023
View reviewed changes
@orangewolf orangewolf marked this pull request as draft July 28, 2023 17:01
@orangewolf
Copy link
Member Author

converted this to a draft so it doesnt get merged w/o the specs or being complete

@summer-cook
Copy link
Contributor

converted this to a draft so it doesnt get merged w/o the specs or being complete

I just saw your comment on this - i thought you had said that the ci failure/issues with the internal test app would be out of scope for this ticket? https://assaydepot.slack.com/archives/C0313NK5NMA/p1690416347279759?thread_ts=1690390955.701519&cid=C0313NK5NMA

@summer-cook summer-cook mentioned this pull request Aug 1, 2023
Merged
summer-cook and others added 3 commits August 3, 2023 13:56
@summer-cook
make change to work id so it is a string
ff02139
@jeremyf
♻️ Adding indices to Hyrax::CounterMetrics
348ac58 
Without these indices, what could be a large database could result in
significant performance degradations at query time.  Each of these
fields is very likely to be part of a "WHERE" statement.

Related to:

- #6130
- https://github.com/scientist-softserv/palni-palci/issues/616WIP
@alishaevn
Merge branch 'main' into counter-i-hardly-knew-her
09fbbea
@alishaevn
Copy link
Contributor

converted this to a draft so it doesnt get merged w/o the specs or being complete

I just saw your comment on this - i thought you had said that the ci failure/issues with the internal test app would be out of scope for this ticket? https://assaydepot.slack.com/archives/C0313NK5NMA/p1690416347279759?thread_ts=1690390955.701519&cid=C0313NK5NMA

@orangewolf 👆🏾 trying to get an idea of where this ticket stands.

@alishaevn alishaevn mentioned this pull request Aug 10, 2023
Closed
3 tasks
@alishaevn alishaevn marked this pull request as ready for review August 28, 2023 16:22
@jeremyf jeremyf merged commit a1d7f81 into main Aug 28, 2023
@jeremyf jeremyf deleted the counter-i-hardly-knew-her branch August 28, 2023 16:37
@dlpierce dlpierce added the notes-minor Release Notes: Non-breaking features label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alishaevn alishaevn alishaevn approved these changes

Assignees
No one assigned
Labels
notes-minor Release Notes: Non-breaking features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@orangewolf @summer-cook @alishaevn @jeremyf @dlpierce