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

fix: Updated logging in the client pool and client factory to log information about the required transport and actual transport used. #1853

Merged

Conversation

MarkDuckworth
Copy link
Contributor

@MarkDuckworth MarkDuckworth commented May 30, 2023

No description provided.

…ion about the required transport and actual transport used.
@MarkDuckworth MarkDuckworth requested review from a team as code owners May 30, 2023 19:35
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label May 30, 2023
@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcf bot commented May 30, 2023

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label May 30, 2023
null,
'Initialized Firestore GAPIC Client (useFallback: %s)',
useFallback
);
Copy link
Contributor

Choose a reason for hiding this comment

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

not part of this PR, but the 575 above

const useFallback = !this._settings.preferRest || requiresGrpc ? false : 'rest';

is... not great. Took me a second to wrap my head around it, partly because the logic is negated IMO. I think writing it this way makes more sense:

const useFallback = this._settings.preferRest && !requiresGrpc ? 'rest' : false;

@ehsannas
Copy link
Contributor

Also, conventionalcommits bot is failing. Need to update the PR title.

@MarkDuckworth MarkDuckworth changed the title Updated logging in the client pool and client factory fix: Updated logging in the client pool and client factory to log information about the required transport and actual transport used. May 30, 2023
@MarkDuckworth MarkDuckworth added the automerge Merge the pull request once unit tests and other checks pass. label May 30, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit fe03d02 into main May 30, 2023
14 checks passed
@gcf-merge-on-green gcf-merge-on-green bot deleted the markduckworth/client-pool-transport-logging branch May 30, 2023 20:42
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 30, 2023
gcf-owl-bot bot added a commit that referenced this pull request Sep 12, 2023
Revert "fix: update sample directory link (#1853)"

This reverts commit 270f3abf028c63ba3668380329ff532c1c21ed93.

Source-Link: googleapis/synthtool@a9808dd
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:2feb2cd10b64d2b1623f7c6353aa87449e46aa1548024c657c3baf974d37cf38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants