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.

Sign up for GitHub

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

Jump to bottom

[VertexAI] Allow user to set location #8219

Merged
merged 3 commits into from May 6, 2024
Merged

[VertexAI] Allow user to set location #8219

merged 3 commits into from May 6, 2024

Conversation

hsubox76
Copy link
Contributor

@hsubox76 hsubox76 commented May 3, 2024

Revert the location part of #8124 and allow user to set location again.

Also get rid of some TS errors on SafetyRatings in tests.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 3, 2024 •
edited

⚠️ No Changeset found

Latest commit: a49dc31

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hsubox76 hsubox76 changed the title Allow user to set location [VertexAI] Allow user to set location May 3, 2024
@hsubox76 hsubox76 marked this pull request as ready for review May 3, 2024 22:19
@hsubox76 hsubox76 requested a review from a team as a code owner May 3, 2024 22:19
@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 3, 2024 •
edited

Size Report 1

Affected Products

  • @firebase/vertexai

    TypeBase (b981a45)Merge (85ee540)Diff
    browser24.4 kB24.6 kB+216 B (+0.9%)
    main25.0 kB25.2 kB+216 B (+0.9%)
    module24.4 kB24.6 kB+216 B (+0.9%)

  • firebase

    TypeBase (b981a45)Merge (85ee540)Diff
    firebase-vertexai.js18.8 kB18.9 kB+149 B (+0.8%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/sqXlxekHIQ.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 3, 2024 •
edited

Size Analysis Report 1

Affected Products

  • @firebase/vertexai

    • BlockReason

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.41 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • ChatSession

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size14.0 kB14.1 kB+118 B (+0.8%)
      size-with-ext-deps32.4 kB32.5 kB+118 B (+0.4%)

    • FinishReason

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.41 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • FunctionCallingMode

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.42 kB3.54 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • FunctionDeclarationSchemaType

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.43 kB3.55 kB+118 B (+3.4%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • GenerativeModel

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size16.2 kB16.3 kB+118 B (+0.7%)
      size-with-ext-deps34.6 kB34.7 kB+118 B (+0.3%)

    • HarmBlockMethod

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.42 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • HarmBlockThreshold

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.42 kB3.54 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • HarmCategory

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.41 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • HarmProbability

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.42 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • HarmSeverity

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.41 kB3.53 kB+118 B (+3.5%)
      size-with-ext-deps21.1 kB21.2 kB+118 B (+0.6%)

    • POSSIBLE_ROLES

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.46 kB3.58 kB+118 B (+3.4%)
      size-with-ext-deps21.1 kB21.3 kB+118 B (+0.6%)

    • getGenerativeModel

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size16.2 kB16.4 kB+118 B (+0.7%)
      size-with-ext-deps34.7 kB34.8 kB+118 B (+0.3%)

    • getVertexAI

      Size

      TypeBase (b981a45)Merge (85ee540)Diff
      size3.54 kB3.71 kB+162 B (+4.6%)
      size-with-ext-deps28.5 kB28.7 kB+162 B (+0.6%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/wUQ6fJBOOR.html

DellaBitta
DellaBitta requested changes May 4, 2024
View reviewed changes
packages/vertexai/src/index.ts Outdated Show resolved Hide resolved
packages/vertexai/src/service.test.ts
);
expect(vertexAI.location).to.equal('somewhere');
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to include both sides of the fence. 👍

dlarocque
dlarocque approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@dlarocque dlarocque left a comment

Choose a reason for hiding this comment

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

LGTM after @ DellaBitta's request changes

@hsubox76 hsubox76 merged commit 964bd26 into ch-vertex-feature May 6, 2024
39 of 41 checks passed
@hsubox76 hsubox76 deleted the ch-vertex-location branch May 6, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DellaBitta DellaBitta DellaBitta approved these changes

@dlarocque dlarocque dlarocque approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hsubox76 @google-oss-bot @DellaBitta @dlarocque