Skip to content

Blocking auth Python samples#1073

Merged
egilmorez merged 1 commit intopythonfrom
py-auth
Apr 29, 2023
Merged

Blocking auth Python samples#1073
egilmorez merged 1 commit intopythonfrom
py-auth

Conversation

@kevinthecheung
Copy link
Copy Markdown
Contributor

No description provided.

return


# TODO: Should really be non-blocking or client-side call.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should include this example.

  1. There's no need for this to be blocking, AFAIK.
  2. The (Python) Admin SDK intentionally doesn't support sending emails from Firebase, so there's a big hand-wave.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you mean, completely not cover Python support in https://firebase.google.com/docs/functions/beta/auth-blocking-events?

You're the expert on that file, so we will be directed by your opinion I think. All the Node snippets are hardcoded, fwiw.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we should remove or rework this example altogether, for both Python and Node, for the reasons given above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Roger that! Saw your other PR (thumbs up emoji).

@jhuleatt
Copy link
Copy Markdown
Collaborator

Hey @kevinthecheung, should this PR be closed in favor of #1074, or should I review this too?

@kevinthecheung
Copy link
Copy Markdown
Contributor Author

Hey @kevinthecheung, should this PR be closed in favor of #1074, or should I review this too?

Please review.

The docs use snippets from both. I just split off #1074 because it has several moving parts.

Copy link
Copy Markdown
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

Lets get these merged, thanks

@egilmorez egilmorez merged commit 715e909 into python Apr 29, 2023
Comment on lines +218 to +219
firestore_client: google.cloud.firestore.Client = firestore.client()
doc = firestore_client.collection("banned").document(email).get()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we use the Firebase Admin SDK here instead?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We do, that's just for typing/Intellisense

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