-
Notifications
You must be signed in to change notification settings - Fork 101
feat: Integrate Redis client in Card Service for (requestId, posServiceHost) mapping #3524
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
Conversation
🚀 Performance Test ResultsTest Configuration:
Test Metrics:
📜 Logs |
| ): Promise<string> => { | ||
| const POSHost = await deps.redis.get(requestId) | ||
| if (!POSHost) { | ||
| deps.logger.error(`No POS found for requestId: ${requestId}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: With the pino logger we usually pass in an object with relevant data, then the message - for example:
deps.logger.error({ requestId }, 'No POS found for requestId')
| requestId: string, | ||
| POSHost: string | ||
| ) => { | ||
| await deps.redis.set(requestId, POSHost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add an expiry as well. Maybe like five minutes or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added 5 minutes for now, maybe we can have it configurable in the future - i dont think anybody will wait 5 minutes at a pos for a response
There was a problem hiding this 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. I would consider merging in the changes from #3533 or waiting for it to be merged into the feature branch to avoid having to redo the tests for this feature in another PR.
…ceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]>
…ceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]>
…ceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]>
* feat: created the backbone for the card service (#3508) * Created the backbone for the card service * Format fix * feat: add card service to docker compose --------- Co-authored-by: Nathan Lie <[email protected]> * feat: Integrate Redis client in Card Service for (requestId, posServiceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]> * feat(point-of-sale): added route for registering a POS device (#3555) * Route for registering a POS device * Fixed issue addressed in comments, tried to fix the port issue of jest * Changed test container port to 0, format fix * feat(backend): publish webhooks to POS service if applicable * feat: add column to incoming payment * fix: tests * feat: add warn log * fix: better logger requirements for finalizing webhook recipients * fix: make open payments default reason for incoming payment initialization * fix: improve optional env var function * fix: warn log, better optional env, backfill migration, tests * fix: rebase bugs * chore: regenerate lockfile * chore: regenerate gql * fix: tests * fix: remove metadata; better tests * fix: build errors * fix: tests, add type file --------- Co-authored-by: oana-lolea <[email protected]> Co-authored-by: zeppelin44 <[email protected]> Co-authored-by: Antoniu Neacsu <[email protected]>
…ceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]>
…backend (#3613) * feat: created the backbone for the card service (#3508) * Created the backbone for the card service * Format fix * feat: add card service to docker compose --------- Co-authored-by: Nathan Lie <[email protected]> * feat: initialize POS service (#3509) * feat(wip): pos service * Completed POS service init * Added docker files and missing scripts * Removed editor code after handling conflicts * Updated docker-compose for cloud 9 localenv and removed unnecessary telemetry command from dockerfile * Updated docker compose files for c9 and hlb * Updated ports and env var names from config * fix: install ts-node-dev, add main script to index.ts --------- Co-authored-by: Nathan Lie <[email protected]> * feat: Integrate Redis client in Card Service for (requestId, posServiceHost) mapping (#3524) * Integrate Redis client in Card Service for (requestId, posServiceHost) mapping * remove unused dep * prettier fix * Separate logging params from the message itself * ttl * prettier fix * Rewrite redis service tests to use testcontainers instead of mocks --------- Co-authored-by: Antoniu Neacsu <[email protected]> * feat(point-of-sale): added route for registering a POS device (#3555) * Route for registering a POS device * Fixed issue addressed in comments, tried to fix the port issue of jest * Changed test container port to 0, format fix * feat(wip): merge pos-card-services * feat(point-of-sale): handle incoming payment completed webhooks from backend * fix: typo, expose incoming payment url in gql * fix: typo & unintended changes * chore: regenerate gql * feat: clean up request map if wehbook times out --------- Co-authored-by: oana-lolea <[email protected]> Co-authored-by: zeppelin44 <[email protected]> Co-authored-by: Antoniu Neacsu <[email protected]>
No description provided.