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

[SOA-704] feat(app): Add Session Pool Options to the Spanner connection #11

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

MuFa117
Copy link

@MuFa117 MuFa117 commented Aug 23, 2023

Description of change

Typeorm does not offer sessionPool options to be passed through the connection and uses the default ones. We would want to eventually control them from the client, so this would add this opportunity.

Options based on https://github.com/googleapis/nodejs-spanner/blob/main/src/session-pool.ts#L168

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@MuFa117 MuFa117 changed the title Add Session Pool Options to the Spanner connection [SOA-589] feat(app): Add Session Pool Options to the Spanner connection Aug 23, 2023
Copy link

@javierdelafuentesales javierdelafuentesales 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, were you able to test it?

@MuFa117
Copy link
Author

MuFa117 commented Aug 24, 2023

Looks good, were you able to test it?

I tested it locally and all was good :) (I updated the built code within node_modules)

@Xevi
Copy link

Xevi commented Aug 24, 2023

Shall we test it on staging-eu with real application? Before we merge this one??

@javierdelafuentesales
Copy link

javierdelafuentesales commented Aug 24, 2023

Shall we test it on staging-eu with real application? Before we merge this one??

@Xevi How do you test it on staging-eu without merging it? I think that first we need to get the new version.

@MuFa117
Copy link
Author

MuFa117 commented Aug 24, 2023

@Xevi merging this PR would still be fine until we bump up client’s version, so beta/prod is going to be fine, but I’m gonna bump the version on onair-api and deploy to staging-eu for sure if that’s what you were talking about

@Xevi
Copy link

Xevi commented Aug 24, 2023

We could always modify the action to make this branch to be able to get built. But should be fine if we are not using this version to anything in prod

@MuFa117 MuFa117 merged commit b9aca7e into main Aug 24, 2023
1 check passed
@MuFa117 MuFa117 deleted the feat/soa/589-add-pool-size branch August 24, 2023 16:44
@MuFa117 MuFa117 changed the title [SOA-589] feat(app): Add Session Pool Options to the Spanner connection [SOA-704] feat(app): Add Session Pool Options to the Spanner connection Aug 31, 2023
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