Skip to content

birkbeckctp/janeway#3657 adds a report for repository metrics.#37

Merged
ajrbyers merged 4 commits into
masterfrom
3657-preprint_metrics_relocation
Mar 8, 2024
Merged

birkbeckctp/janeway#3657 adds a report for repository metrics.#37
ajrbyers merged 4 commits into
masterfrom
3657-preprint_metrics_relocation

Conversation

@ajrbyers
Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member

@mauromsl mauromsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small comments inline.
There is the larger question of performance, but that's not going to be worse than what we had before. Since this is now in the reporting plugin it is probably worth checking how it performs in staging/prod environments before we action on it.

Comment thread logic.py Outdated
Comment thread logic.py Outdated
@ajrbyers
Copy link
Copy Markdown
Member Author

Couple of small comments inline. There is the larger question of performance, but that's not going to be worse than what we had before. Since this is now in the reporting plugin it is probably worth checking how it performs in staging/prod environments before we action on it.

I've made changes as requested. During dev I tested this with 5,314,070 preprint access records and it was reasonably fast. Unfortunately changing to filter by __date seems to have caused a slight performance hit, it takes 2116.88ms for the query now and it was previously < 1000ms.

@ajrbyers ajrbyers requested a review from mauromsl February 27, 2024 13:44
Copy link
Copy Markdown
Member

@mauromsl mauromsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor suggestion regarding the new DateForm to make usage a bit cleaner.

There is also the question of performance, but I think we'll need to get results from a real system before we can optimize for performance.

Comment thread views.py Outdated
@ajrbyers ajrbyers requested a review from mauromsl March 4, 2024 10:59
@joemull joemull self-requested a review March 5, 2024 14:23
@ajrbyers ajrbyers merged commit 18f34ba into master Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants