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

Explicit Caching patch #377

Merged
merged 37 commits into from
Jun 13, 2024
Merged

Explicit Caching patch #377

merged 37 commits into from
Jun 13, 2024

Conversation

mayureshagashe2105
Copy link
Contributor

@mayureshagashe2105 mayureshagashe2105 commented Jun 5, 2024 •
edited
Loading

follow-on PR for #355

  • Patch .update
  • Remove __new__ constructs, and add public property to access cached_content on GenerativeModel class.
  • Reomve auto cache deletion.
  • Remove name param
  • Handle ttl and expire_time separately
  • Implict role assignment as user if the content is of type list[str] | str
  • Minor fixes

@mayureshagashe2105
Squashed commit of the following:
5824d8e 
commit acb3806
Author: Mayuresh Agashe <magashe@google.com>
Date:   Wed Jun 5 00:51:30 2024 +0000

    fix update method

    Change-Id: I433c25b2d80cdf6e483b59f61ff29bb8d2dc6595

commit fb9995c
Merge: 4627fe1 7b9758f
Author: Mark Daoust <markdaoust@google.com>
Date:   Tue Jun 4 09:55:38 2024 -0700

    Merge branch 'main' into caching

    Change-Id: I2bade6b0099f12dd37a24fe26cfda1981c58fbc0

commit 4627fe1
Author: Mark Daoust <markdaoust@google.com>
Date:   Tue Jun 4 09:54:31 2024 -0700

    use preview build

    Change-Id: Ic1cd4fc28f591794dc5fbff0647a00a77ea7f601

commit 8e86ef1
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 30 16:18:22 2024 +0000

    Refactor for genai.protos module

    Change-Id: I2f02d2421d7303f0309ec86f05d33c07332c03c1

commit 82d3c5a
Merge: bf6551a f08c789
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 30 15:57:27 2024 +0000

    Merge branch 'main' of https://github.com/mayureshagashe2105/generative-ai-python into caching

    Change-Id: Id2b259fe4b2c91653bf5e4d5e883f556366d8676

commit bf6551a
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 11:26:03 2024 +0000

    Fix types

    Change-Id: Id3e7316562f4029e5b7409ae725bb66e2207f075

commit 67472d3
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 11:26:03 2024 +0000

    Fix types

    Change-Id: Id3e7316562f4029e5b7409ae725bb66e2207f075

commit a1c8c72
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 11:15:15 2024 +0000

    Fix docstrings

    Change-Id: I6020df4e862a4f1d58462a4cd70876a8448293cf

commit f48cedc
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 11:13:44 2024 +0000

    Fix types

    Change-Id: Ia4bf6b936fab4c1992798c65cff91c15e51a92c0

commit 645ceab
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 05:54:26 2024 +0000

    blacken

    Change-Id: I4e073d821d29eea30801bdb7e2a8dc01bb7d6b9a

commit 17372e3
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 05:54:06 2024 +0000

    Add 'cached_content' to GenerativeModel's repr

    Change-Id: I06676fad23895e3e1a6393baa938fc1f2df57d80

commit d1fd749
Author: Mayuresh Agashe <magashe@google.com>
Date:   Mon May 27 05:04:43 2024 +0000

    Add type-annotations to __new__ to fix pytype checks

    Change-Id: I6c69c036e54d56d18ea60368fa0a1dcda2d315fd

commit f37df8c
Author: Mayuresh Agashe <magashe@google.com>
Date:   Sun May 26 06:51:54 2024 +0000

    mark name as OPTIONAL for CachedContent creation

    If not provided, the name will be randomly generated

    Change-Id: Ib95fbafd3dfe098b43164d7ee4d6c2a84b0aae2e

commit 59663c8
Author: Mayuresh Agashe <magashe@google.com>
Date:   Fri May 24 10:22:08 2024 +0000

    Add tests

    Change-Id: I249188fa585bd9b7193efa48b1cfca20b8a79821

commit e1d8c7a
Author: Mayuresh Agashe <magashe@google.com>
Date:   Fri May 24 10:21:42 2024 +0000

    Validate name checks for CachedContent creation

    Change-Id: Ie41602621d99ddff6404c6708c7278e0da790652

commit 2cde1a2
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 23 18:09:14 2024 +0000

    fix tests

    Change-Id: I39f61012f850a82e09a7afb80b527a0f99ad0ec7

commit d862dae
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 23 18:09:14 2024 +0000

    fix tests

    Change-Id: I39f61012f850a82e09a7afb80b527a0f99ad0ec7

commit d35cc71
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 23 23:12:38 2024 +0530

    Improve tests

commit e65d16e
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 23 23:12:05 2024 +0530

    blacken

commit cfc936e
Author: Mayuresh Agashe <magashe@google.com>
Date:   Thu May 23 23:10:16 2024 +0530

    Stroke out functional approach for CachedContent CURD ops

commit afd066d
Merge: 6fafe6b 0dca4ce
Author: Mayuresh Agashe <magashe@google.com>
Date:   Wed May 22 23:10:20 2024 +0530

    Merge branch 'main' into caching

commit 6fafe6b
Author: Mayuresh Agashe <magashe@google.com>
Date:   Wed May 22 10:49:35 2024 +0530

    rename get_cached_content to get

commit a4ac7a5
Merge: f13228d f987fde
Author: Mayuresh Agashe <magashe@google.com>
Date:   Tue May 21 23:32:41 2024 +0530

    Merge branch 'main' into caching

commit f13228d
Author: Mayuresh Agashe <magashe@google.com>
Date:   Fri Apr 26 16:54:09 2024 +0000

    *Inital prototype for explicit caching

    *Add basic CURD support for caching

    *Remove INPUT_ONLY marked fields from CachedContent dataclass

    *Rename files 'cached_content*' -> 'caching*'

    *Update 'Create' method for explicit instantination of 'CachedContent'

    *Add a factory method to instatinate model with `CachedContent` as
    its context

    *blacken

    *Add tests

    Change-Id: I694545243efda467d6fd599beded0dc6679b727d

Change-Id: I7b14d94f729953294780815f4c496888bb2ad46f
@mayureshagashe2105
Remove auto cache deletion
3d8ff35 
Change-Id: I4658e1c57f967faeb3945dffef0181a456d65370
@mayureshagashe2105
Rename _to_dict --> _get_update_fields
4c5a31b 
Change-Id: I3c92c65e8e5b215e98c1ac0eea6db033166dec78
@mayureshagashe2105
Fix tests
b6d440f 
Change-Id: Id36d7606e13d15caf6870f29a108944c7f36eaeb
@mayureshagashe2105
Set 'CachedContent' as a public property
e53dbab 
Remove __new__ construct

Change-Id: Ie4f5527270be90730341b6c3b67de71b9b6e9c5c
@mayureshagashe2105
blacken
f53a7c6 
Change-Id: I12498213a7fc2b257827ab0df87c6913e04cad25
@mayureshagashe2105 mayureshagashe2105 force-pushed the magashe-caching-patch-1 branch from 883b500 to f53a7c6 Compare June 5, 2024 02:18
@mayureshagashe2105 mayureshagashe2105 marked this pull request as ready for review June 5, 2024 02:30
@github-actions github-actions bot added status:awaiting review PR awaiting review from a maintainer component:python sdk Issue/PR related to Python SDK labels Jun 5, 2024
MarkDaoust
MarkDaoust reviewed Jun 5, 2024
View reviewed changes
Copy link
Collaborator

@MarkDaoust MarkDaoust left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It turns out name is not settable. They're talking about adding display_name. But IDK if it'll make the initial launch.

    def create(
        cls,
        model: str,
        name: str | None = None,

@mayureshagashe2105
Copy link
Contributor Author

Yeah, It simply overrides the name.

@mayureshagashe2105
set 'role=user' when content is passed as a str (#4)
02cba55 
'to_content' method assigns a default 'role=user' to all the contents passed as a string

Change-Id: I748514a7839b7f1d36150b879c3d1464ca9e11ba
@mayureshagashe2105 mayureshagashe2105 force-pushed the magashe-caching-patch-1 branch from 2bb9aa6 to 02cba55 Compare June 5, 2024 19:32
MarkDaoust
MarkDaoust requested changes Jun 5, 2024
View reviewed changes
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Show resolved Hide resolved
google/generativeai/types/caching_types.py Outdated Show resolved Hide resolved
@mayureshagashe2105
Handle ttl and expire_time separately
4c495ef 
Change-Id: If9c6f04fe8d419828e3efd2249f0698bca4d5bdc
@mayureshagashe2105
Remove name param
0f5f8eb 
Change-Id: I40fe7c8fafdb014fb9c7e74956452aca9a666641
MarkDaoust
MarkDaoust requested changes Jun 6, 2024
View reviewed changes
google/generativeai/caching.py Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/types/content_types.py Outdated Show resolved Hide resolved
tests/test_answer.py Outdated Show resolved Hide resolved
tests/test_caching.py Show resolved Hide resolved
@mayureshagashe2105
Update docstrs and error messages
42b1e35 
Change-Id: I111a1218a7d9783d494b84f0a11cb3b76c7ad9da
@mayureshagashe2105
Update model name to gemini-1.5-pro for caching tests
e4648a7 
Change-Id: Ibb1f75c409afaac124ef70232be71e3a882f6015
@mayureshagashe2105
Merge branch 'magashe-caching-patch-1' of https://github.com/mayuresh…
f2b495f 
…agashe2105/generative-ai-python into magashe-caching-patch-1

Change-Id: I525dc7b1ec4e3f9eac59ed13a52d72b9fadda5a6
@mayureshagashe2105
Remove dafault ttl assignment
f715ecb 
Let the API set the dafault

Change-Id: Id8d125a085ed27229ddb78d5812ed5b5ad39227b
@mayureshagashe2105
blacken
a576166 
Change-Id: I1d7fe0ec422589e237502b0eda687cf81ef21a21
@mayureshagashe2105
Remove client arg
5e9b14b 
Change-Id: I17f05a90a1514f404dd3527c0db1ce6147d2c47a
@mayureshagashe2105
Add 'usage_metadata' param to CachedContent class
6ccee3e 
Change-Id: Ic527c157bc2cd114948b73a8f1832c21dd61b52e
@mayureshagashe2105
Add 'display_name' to CachedContent class
3de6909 
Change-Id: Id0a9be9d1bfdb94dc9d5c4fc7af9dee89e5365a4
@mayureshagashe2105 mayureshagashe2105 mentioned this pull request Jun 11, 2024
Merged
@MarkDaoust
update generativelanguage version, fix tests
7fccb32 
Change-Id: I0acc57853ab7dde863bbbe4b30ae3957e6ec3d11
@MarkDaoust
format
2fabe67 
Change-Id: Ib2e9a16aaa989021d3498f3e59f9983560919159
@MarkDaoust
fewer automatic 'role' insertions
7d14bb1 
Change-Id: I0752741532a451f8720fa5e110e68f0b4e66cc4b
@MarkDaoust
cleanup
3982b48 
Change-Id: I151a809f6d079b8e4b0ed30d1153a638c98cacfd
@MarkDaoust
Wrap the proto
940834a 
Change-Id: I14b4c54652fb51b867fb43d4b3e9091e6eaccd4e
mayureshagashe2105
mayureshagashe2105 commented Jun 12, 2024
View reviewed changes
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
MarkDaoust and others added 12 commits June 12, 2024 13:09
@MarkDaoust @mayureshagashe2105
Apply suggestions from code review
4a5229e 
Co-authored-by: Mayuresh Agashe <magashe@google.com>
@MarkDaoust
fix
c039644 
Change-Id: I381029fc8fc13c39e432b39084fc8feba305514e
@MarkDaoust
format
fc767a1 
Change-Id: I8e0b44aebc102d3b2afb27a422c4d70d6c99d5d2
@MarkDaoust
cleanup
75cc224 
Change-Id: I024733b53cede5bfdf957ce7e56d6ad01fd4b2bf
@MarkDaoust
update version
19f0384 
Change-Id: Ic95dffb3e945e31adc0d98787942d27289512b8a
@MarkDaoust
fix
d438860 
Change-Id: I6ffdabbddf0e803606b3638521ebfeb6796d2e4b
@MarkDaoust
typing
aa12c3d 
Change-Id: I629d4d111f0e640f4f4bf602ea33f70fdc9ca3e4
@MarkDaoust
Merge branch 'main' into magashe-caching-patch-1
cc54a87 
Change-Id: I928933edc82bcfb42f2ade6bfb49c5eae2a9ee3b
@mayureshagashe2105
Simplify update method
a6f4355 
Accept kwargs instead of dict of updates and construct protos using kwargs

Change-Id: I7858d585b1aa6b965134e2fb90adff737172af92
@mayureshagashe2105
Add repr to CachedContent
1c77da4 
Change-Id: Id4ec78ebf9d6e96f22f6bf37fc4509268fa552f4
@mayureshagashe2105
cleanup
0bac36e 
Change-Id: I684b46f881735bceb3f9e09d8573721ddb29f98a
@mayureshagashe2105
blacken
25f4d10 
Change-Id: I773e7a5b8a222c8b4435470cdc2b53be425d95e4
@mayureshagashe2105
Copy link
Contributor Author

@MarkDaoust @markmcd We are good to go with context caching 🥳. Thanks for all the feedback and contributions.

@markmcd can you please review my last commits. Thanks!

markmcd
markmcd previously approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@markmcd markmcd left a comment

Choose a reason for hiding this comment

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

Looks good - just a few questions, hopefully nothing too stupid on my part :) LMK if you'd rather do any of it in a followup too.

google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/caching.py Outdated Show resolved Hide resolved
google/generativeai/generative_models.py Show resolved Hide resolved
google/generativeai/generative_models.py Outdated Show resolved Hide resolved
google/generativeai/version.py Show resolved Hide resolved
@mayureshagashe2105
Apply suggestions from code review
9b48863 
Change-Id: I2a12b9689001bbc41c460db5a9f0e87c77d4caf6
@MarkDaoust MarkDaoust merged commit 23b81d7 into google-gemini:main Jun 13, 2024
7 checks passed
@github-actions github-actions bot removed the status:awaiting review PR awaiting review from a maintainer label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmcd markmcd markmcd approved these changes

@MarkDaoust MarkDaoust MarkDaoust approved these changes

Assignees
No one assigned
Labels
component:python sdk Issue/PR related to Python SDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mayureshagashe2105 @markmcd @MarkDaoust