Skip to content

fix: update environment variable names for AI API configuration in RunAiQuery - #479

Open
satti-hari-krishna-reddy wants to merge 7 commits into
Shuffle:mainfrom
satti-hari-krishna-reddy:onprem-hybrid
Open

satti-hari-krishna-reddy wants to merge 7 commits into
Shuffle:mainfrom
satti-hari-krishna-reddy:onprem-hybrid

Conversation

@satti-hari-krishna-reddy

Copy link
Copy Markdown
Collaborator

No description provided.

@frikky frikky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:)

Comment thread ai.go Outdated
decrypted, err := HandleKeyDecryption([]byte(field.Value), parsedKey)
if err == nil {
curApiKey = string(decrypted)
if !auth.Encrypted {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's probably better here to not fully trust auth.Encrypted, as this can also be controlled by the user (manual override)

You may want to just TRY decrypt, and if it fails, use the raw value.

Comment thread ai.go Outdated
}

// Custom URL must only be used when paired with an API key
if len(curUrl) > 0 && len(curApiKey) == 0 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Weird, wasn't this here already?

Comment thread ai.go

// Checks if cloud sync is set up
if len(org.SyncConfig.Apikey) > 0 {
if len(org.SyncConfig.Apikey) > 0 && org.SyncConfig.AiCloudSync {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is AiCloudSync used for?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

AiCloudSync is a specific parameter introduced by Lalit that acts as a control flag. It requires the user to explicitly enable AI cloud sync. @LalitDeore , mind chiming in here with the exact details or context on this ?

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.

What is AiCloudSync used for?

AiCloudSync is used to track whether user have enabled/disabled the ai execution through cloud sync or not. By default it is off and it can be enable from admin page.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would it be off by default? On by default obviously makes more sense lol

@frikky frikky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Off by default seems wrong

Comment thread ai.go
orgId = os.Getenv("OPENAI_API_ORG")
}
// Fallback only. Using SHUFFLE_AI_* instead of AI_API_KEY so legacy env vars
// don't accidentally block Cloud Gemini. If this is set, it's intentional.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a stupid rationale.

Fixing gemini implementation is the choice here (which I kin of did anyway) 🤦 The fallback and fallback is obviously better as it uses more standard API-keys and such. And most people won't use this shit anyway

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