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

add eras-client for projects stats callouts #227

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

yuenmichelle1
Copy link
Contributor

Adding simple ERAS client for Project Stats Callouts.

I mainly focused on callouts that will be used for project stats (as opposed to using statsClient on PFE).

One thing I did not add for eras client are the future callouts that will be used. Eg. Getting classification counts by user_group (/classifications/user_groups/:id or getting classification count by user /classifications/users/:id). I figured that would be done in a future iteration when we are ready to cross that bridge. (I don't mind adding it on now if we want to have it available in PJC now).

@eatyourgreens
Copy link
Contributor

We have a new API client in the monorepo. Should the Eras client be added to that, so that @mcbouslog can use it in the work that he is doing?

@eatyourgreens
Copy link
Contributor

I think you would just need to copy this file and add erasHost to the config.

https://github.com/zooniverse/front-end-monorepo/blob/master/packages/lib-panoptes-js/src/talkAPI.js

@yuenmichelle1
Copy link
Contributor Author

We have a new API client in the monorepo. Should the Eras client be added to that, so that @mcbouslog can use it in the work that he is doing?

Ah. @eatyourgreens fair point. My main goal of updating PJC is to replace PFE project stats (replacing anything that currently uses the slow running Elasticsearch stats service, and eventually shutting down ES stats service) with ERAS stats callouts.

My plan is to do a minimal lift to do the replacement and then when @mcbouslog is ready he can do update API client in monorepo with any other changes he may need.

Copy link
Contributor

@mcbouslog mcbouslog left a comment

Choose a reason for hiding this comment

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

Code changes LGTM 👍 . Incrementally adding functionality (if needed here, as opposed to an eras client in FEM/panoptes-js) sounds good.

I ran PFE locally and npm linked these changes and confirmed classifications and comments eras requests functioned as expected ✅ .

I'll merge this PR then create a (minor) release unless there's a preference or suggestion otherwise.

@mcbouslog mcbouslog self-assigned this Nov 28, 2023
@eatyourgreens
Copy link
Contributor

eatyourgreens commented Nov 29, 2023

Sounds good to me. You can probably get rid of panoptes-client/lib/stats-client too, as a breaking change.

Something I learnt yesterday: Next.js data-fetching uses fetch in Node and in the browser. So the user-groups work in the monorepo should use fetch, not superagent, to get stats data.

@mcbouslog mcbouslog merged commit 0758126 into main Nov 29, 2023
3 checks passed
@mcbouslog mcbouslog deleted the add-eras-client branch November 29, 2023 15:37
@yuenmichelle1
Copy link
Contributor Author

Code changes LGTM 👍 . Incrementally adding functionality (if needed here, as opposed to an eras client in FEM/panoptes-js) sounds good.

I ran PFE locally and npm linked these changes and confirmed classifications and comments eras requests functioned as expected ✅ .

I'll merge this PR then create a (minor) release unless there's a preference or suggestion otherwise.

@mcbouslog thanks for closing the loop on the changes. You're amazing! 🎉
@eatyourgreens @mcbouslog thanks for review :)

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