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

Get pagination params in handlers from utils #10962

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RyanCarlisle
Copy link

@RyanCarlisle RyanCarlisle commented May 15, 2024

Notes for Reviewers

This PR fixes #10825 : the repetitive logic for extracting page, pagesize, search, offset from API handlers and uses getPaginationParams in utils.go

Signed commits

  • Yes, I signed my commits.

Copy link

welcome bot commented May 15, 2024

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while performing a commit.

Copy link

@RyanCarlisle RyanCarlisle changed the title Feature/ryan carlisle/handlers get pagination params Get pagination params in handlers from utils May 15, 2024
Copy link

@864893031735973 864893031735973 left a comment

Choose a reason for hiding this comment

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

Ok

@vishalvivekm
Copy link
Member

Hey @RyanCarlisle, let's discuss this on today's Meshery Dev meeting at 7:30 PM IST. I am adding this as an agenda item. Please Join https://meet.layer5.io/meshery if you are available during the meet.

Copy link
Contributor

@MUzairS15 MUzairS15 left a comment

Choose a reason for hiding this comment

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

What testing have you performed @RyanCarlisle ?
Did you test the UI by building the server on this PR, did you faced any regression issues or failing APIs or incorrect results?
Did you verified if pagination and search still works fine in the UI.

@MUzairS15
Copy link
Contributor

@Yashsharma1911

@RyanCarlisle
Copy link
Author

RyanCarlisle commented May 22, 2024

@MUzairS15 I have tested a few APIs by building the UI and server with my code changes and the API responses were fine without any issues. However, I could not test it thoroughly as WSL/VIrtual Linux took around 5-6 GB of RAM (plus Google Chrome which took an extra 2 GB, totaling up to 8 GB) which slowed the entire system down, making it a bit difficult for me to continue testing.

@MUzairS15
Copy link
Contributor

Can you list the APIs that you tested?
Are you running ui-dev-server as well?

@RyanCarlisle
Copy link
Author

The commands that I had run were as mentioned in the guide. These were as follows:

  1. make ui-build
  2. make ui-setup
  3. make server
  4. make ui

I have not made a list but I shall try to make a list of the APIs after testing them out again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants