Skip to content

Upgrade sqlalchemy#61

Open
hartikainen wants to merge 5 commits intogoogle-deepmind:mainfrom
hartikainen:upgrade-sqlalchemy
Open

Upgrade sqlalchemy#61
hartikainen wants to merge 5 commits intogoogle-deepmind:mainfrom
hartikainen:upgrade-sqlalchemy

Conversation

@hartikainen
Copy link
Copy Markdown

@hartikainen hartikainen commented Sep 7, 2025

Fixes #60. The tests were created with gemini-cli's help.

Also fixes what I believe could be a big in how the job data is handled. See my comment in-line.

Comment on lines -259 to +264
data = text_format.MessageToBytes(job)
data = text_format.MessageToString(job)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This seemed like a bug to me. The database column for job_data is of type sa.String(255), so I think the data should be formatted as string and not bytes. MessageToString is also in line with what insert_kubernetes_job below does already.

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.

Upgrade sqlalchemy

1 participant