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

feat(router): implement ApiKeyInterface for MockDb #1101

Merged
merged 11 commits into from
May 16, 2023
Merged

feat(router): implement ApiKeyInterface for MockDb #1101

merged 11 commits into from
May 16, 2023

Conversation

derekleverenz
Copy link
Contributor

@derekleverenz derekleverenz commented May 9, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Stores API keys in a vector in MockDB.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

Resolves #996.

How did you test it?

Wrote a unit test (included in the PR, but ignored). It passes locally if I make some unrelated changes to allow the test to run without needing redis, but I have it ignored for now because of that (the tests can probably be removed too, they arent vital just wanted to verify it worked)

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@derekleverenz derekleverenz requested a review from a team as a code owner May 9, 2023 22:50
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Other than that, looks good to me!

Thanks for the PR, @derekleverenz!

crates/router/src/db/api_keys.rs Outdated Show resolved Hide resolved
crates/router/src/db/api_keys.rs Outdated Show resolved Hide resolved
@SanchithHegde SanchithHegde added A-framework Area: Framework C-feature Category: Feature request or enhancement S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 10, 2023
@SanchithHegde SanchithHegde changed the title feat(router): implement apikeyinterface for mockdb feat(router): implement ApiKeyInterface for MockDb May 10, 2023
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Other than that, looks good to me!

Edit: Also, please address the CI checks. It's fine to allow the clippy::unwrap_used lint in the tests module.

crates/router/src/db/api_keys.rs Outdated Show resolved Hide resolved
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Thanks again for the PR, @derekleverenz!

@SanchithHegde SanchithHegde added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 13, 2023
@jarnura jarnura added S-ready-for-merge and removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels May 16, 2023
@jarnura jarnura added this pull request to the merge queue May 16, 2023
Merged via the queue into juspay:main with commit 95c7ca9 May 16, 2023
@NarsGNA
Copy link
Contributor

NarsGNA commented May 17, 2023

@derekleverenz can we have your twitter / LinkedIn handles? Would love to give you a shoutout!

@derekleverenz
Copy link
Contributor Author

Sure! Im @Coaxmetal on twitter and https://www.linkedin.com/in/derek-leverenz-b0787858/ at linkedin

@NarsGNA
Copy link
Contributor

NarsGNA commented May 18, 2023

Thanks! Can you please enable tagging on LinkedIn, we are unable to tag your profile

@derekleverenz
Copy link
Contributor Author

Ah yep, should be enabled now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Area: Framework C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Implement ApiKeyInterface for MockDb
5 participants