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.

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

chore: image default versions #281

Merged
merged 19 commits into from
Jan 23, 2025
Merged

Conversation

afoley587
Copy link
Member

@afoley587 afoley587 commented Jan 17, 2025

Rationale

Changes the teams-do image defaults to the fiftyone-teams-cv-full image.

Changes

Changes the teams-do image defaults to the fiftyone-teams-cv-full image.
Also issues warnings about the new image sizes.

Checklist

  • This PR maintains parity between Docker Compose and Helm

Testing

@afoley587 afoley587 requested a review from ehofesmann January 17, 2025 17:16
@afoley587 afoley587 requested a review from a team as a code owner January 17, 2025 17:16
Copy link
Member

@findtopher findtopher left a comment

Choose a reason for hiding this comment

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

sure

@afoley587 afoley587 mentioned this pull request Jan 17, 2025
@swheaton
Copy link
Contributor

I think only changing the default for teams-do is lower risk. Only adding 2GB to the image for the few customers that have added teams-do so far.

@swheaton
Copy link
Contributor

Also, should get acknowledgement from all of CS before releasing this change (especially if we're gonna change plugins pod also)

Copy link
Member

@ehofesmann ehofesmann left a comment

Choose a reason for hiding this comment

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

Thanks! I agree with stuart that just making this change for teams-do might be the best way to go for now. This is primarily to run torch models which is a very heavy operation that should always be delegated and not run directly in the app/teams-plugins

@afoley587 afoley587 requested a review from ehofesmann January 17, 2025 20:52
@kevin-dimichel
Copy link
Contributor

@afoley587 - Is there a plan to also update the docker teams-do image default?

@afoley587
Copy link
Member Author

@afoley587 - Is there a plan to also update the docker teams-do image default?

Good point, updated as well.

Copy link
Member

@ehofesmann ehofesmann left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @afoley587 !!

@afoley587 afoley587 requested a review from ehofesmann January 22, 2025 16:26
Copy link
Member

@findtopher findtopher left a comment

Choose a reason for hiding this comment

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

bump to 2.5.0

docker/common-services.yaml Outdated Show resolved Hide resolved
docker/docs/upgrading.md Outdated Show resolved Hide resolved
tests/unit/compose/docker-compose-internal-auth_test.go Outdated Show resolved Hide resolved
tests/unit/compose/docker-compose-legacy-auth_test.go Outdated Show resolved Hide resolved
helm/docs/upgrading.md Outdated Show resolved Hide resolved
Copy link
Member

@ehofesmann ehofesmann left a comment

Choose a reason for hiding this comment

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

Proposed some slight clarifications, otherwise looks good!

afoley587 and others added 5 commits January 23, 2025 09:22
Co-authored-by: Eric Hofesmann <[email protected]>
Signed-off-by: afoley587 <[email protected]>
Co-authored-by: Eric Hofesmann <[email protected]>
Signed-off-by: afoley587 <[email protected]>
Co-authored-by: Eric Hofesmann <[email protected]>
Signed-off-by: afoley587 <[email protected]>
Co-authored-by: Eric Hofesmann <[email protected]>
Signed-off-by: afoley587 <[email protected]>
@afoley587 afoley587 merged commit d368c8e into release/v2.5.0 Jan 23, 2025
6 checks passed
@afoley587 afoley587 deleted the afoley/change-image-defaults branch January 23, 2025 15:03
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.

5 participants