quarkiverse / quarkus-google-cloud-services

Google Cloud Services Quarkus Extensions

https://docs.quarkiverse.io/quarkus-google-cloud-services/main/index.html

Apache License 2.0

53 stars 34 forks source link

Provides Firebase Admin services #543

Open loicmathieu opened 11 months ago

loicmathieu commented 11 months ago

Provides Firebase Admin services

jfbenckhuijsen commented 1 month ago

We don't already have devservices for the firebase admin emulator right?

loicmathieu commented 1 month ago

No, not yet ;)

jfbenckhuijsen commented 1 month ago

gotcha ;)

jfbenckhuijsen commented 1 month ago

@loicmathieu Question: working on this topic. The issue I see is that the Firebase emulator encapsulates both the PubSub and the Firestore emulator. I.e. if we run the Firebase emulator, but also have the DevService for PubSub enabled, I'd say we want the Firebase emulator to handle both and not run that separate container.

Currently I see multi possible approaches to this:

Add a config property use the Firebase emulator (quarks.google.cloud.firebbase.devservice.enabled = true or something). If this is set, dont' run the PubSub (and Firestore and ...) services. Would introduce some coupling at the -deployment level modules to get that hooked up.
Ditch the existing PubSub and Firestore emulators and always run the Firebase emulator (adjusting the --only flag accordingly). Downside of this is that the imageName config property would be a bit hard to support. So that would imply a breaking change in the config.
....?

your thoughts?

loicmathieu commented 1 month ago

@jfbenckhuijsen all deveservices can be disabled by config properties (if not, it's a bug).

To keep things simple, I would only run the firebase emulator from the firebase extension if possible so that there are no coupling between extensions.

jfbenckhuijsen commented 1 month ago

Yeah true, that I got, the issue is the firebase emulator is not just a firebase-auth emulator, it packs a whole lot of emulators. And they also have some interaction when packed together I think (e.g. hosting and functions forwarding rules) which only work when run as a whole.

The current setup I have just lets you specify you want a pub-sub emulator, and the logic internally automagically figures out if you want the firebase-packed one, or the plain one. Ill create a working PR, easier to discuss probably

jfbenckhuijsen commented 3 weeks ago

706 is the ground work for this change, without the work needed to exclude the pubsub or Firestore emulators in case the Firebase emulator is running.

707 is a way to disable the Pubsub/Firestore emulators using MP Config properties which are automatically set when the Firebase module is included. The assumption is in case that module is present, you want to use the Firebase version.

This way, we just have a config dependency between the Pubsub/Firestore modules and the Firebase module, not any runtime or dev-time dependency. Lemme know what you think about this setup. Seems cleanest, as the user can also override this and still use the non-Firebase versions if needed.

Note that these all depend on #705 which isn't ready yet, and I'll need to add some more testing, so don't merge just yet.

loicmathieu commented 3 weeks ago

@jfbenckhuijsen I just merged https://github.com/quarkiverse/quarkus-google-cloud-services/pull/705, I didn't saw that you considered it non-ready. If I need to revert it, please tell me. In those case, better to open it in Draft (or at least add some comment).

For the other one, they are huge, I'll need some time to review them.

jfbenckhuijsen commented 3 weeks ago

No worries, this one is fine, the others are work in progress so don't merge those just yet.

706 and #707 are marked as draft.

As for the size, lemme know what you think about the way to handle that conflict as indicated in #707 that one in itself is not that large.

There is still some work in getting it to actually work, but that's mostly work for #706, not #707.

jfbenckhuijsen commented 1 week ago

@loicmathieu quite a bit of progress. Current state is that the code works and should be ready for an initial review.

It's not yet feature complete fully yet, but the additions are mostly convenience stuff, not the core of the changes.

Also spend some time in cleaning and splitting the code, so it should be a bit easier to review (mind you, it's still quite large).

Current features planned for now are:

Support for storage rules/firestore rules/firestore indexes
Support for including your own custom firebase json instead of the auto-generated one. This should also auto-configure the extension based on the firebase.json file, so you end up with a real nice integrated setup.

However, as the current PR is already quite large, lets focus on getting this part reviewed and merged first, I'll add those changes to a separate PR.

Please focus on #707, that one is the complete one.

jfbenckhuijsen commented 1 week ago

@loicmathieu btw if I can do anything to make the review easier for you, lemme know

loicmathieu commented 1 week ago

I'll try to review your PRs soon.