Add FirebaseAIConfig support #15099
Add FirebaseAIConfig support #15099
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback. |
| code: .appCheckTokenFetchFailed, | ||
| "Missing getLimitedUseToken() function, but requireLimitedUseTokens was enabled." | ||
| ) | ||
| // falls back to standard token |
There was a problem hiding this comment.
Should we fall back or throw an error here? I feel like throwing an error might make more sense, since the developer is explicitly opting in to using limitedUseTokens (especially considering the require prefix). I used a log for connivence first though, wdyt @andrewheard?
Generated by 🚫 Danger |
| @@ -86,13 +87,14 @@ public final class GenerativeModel: Sendable { | |||
| toolConfig: ToolConfig? = nil, | |||
| systemInstruction: ModelContent? = nil, | |||
| requestOptions: RequestOptions, | |||
| urlSession: URLSession = GenAIURLSession.default) { | |||
| urlSession: URLSession = GenAIURLSession.default, aiConfig: FirebaseAIConfig) { | |||
There was a problem hiding this comment.
I expected this to go on a new line, lets see if CI complains. If it doesn't, I'll just fix it manually.
| @@ -30,9 +30,12 @@ struct GenerativeAIService { | |||
|
|
|||
| private let urlSession: URLSession | |||
|
|
|||
| init(firebaseInfo: FirebaseInfo, urlSession: URLSession) { | |||
| private let aiConfig: FirebaseAIConfig | |||
There was a problem hiding this comment.
This could also be propagated down via FirebaseInfo, but I wasn't sure if that was desirable. cc @andrewheard
| /// | ||
| /// Can be set when creating a ``FirebaseAI.Config``. | ||
| @available(iOS 15.0, macOS 12.0, macCatalyst 15.0, tvOS 15.0, watchOS 8.0, *) | ||
| public struct AppCheckOptions: Sendable, Hashable, Encodable { |
There was a problem hiding this comment.
Sendable conformance looks good. Why the Hashable and Encodable conformance?
| ) | ||
| } | ||
|
|
||
| /// Class to enable FirebaseAI to register via the Objective-C based Firebase component system | ||
| /// to include FirebaseAI in the userAgent. | ||
| @objc(FIRVertexAIComponent) class FirebaseVertexAIComponent: NSObject {} | ||
|
|
||
| /// Configuration options for ``FirebaseAI``, which persists across all models. | ||
| @available(iOS 15.0, macOS 12.0, macCatalyst 15.0, tvOS 15.0, watchOS 8.0, *) | ||
| public struct Config: Sendable, Hashable, Encodable { |
There was a problem hiding this comment.
Same here w.r.t. the Hashable and Encodableconformances.
Per b/430576238,
This adds support for the
FirebaseAIConfigstruct, alongside theAppCheckOptionsstruct. ThelimitedUseTokensaspect is implemented, and has been documented as well.This feature still needs to go through API review, so it should not be merged yet.
Tests will also need to be added once the change it approved.
e: Note that no changelog entry is added yet, as I'm unsure when this will be merged. But a changelog entry will need to be added before it is merged.