-
Notifications
You must be signed in to change notification settings - Fork 1.2k
TF-2.16 test modification and handling #4830
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
Conversation
tests/conftest.py
Outdated
@@ -555,6 +556,11 @@ def tf_full_version(tensorflow_training_latest_version, tensorflow_inference_lat | |||
Fixture exists as such, since TF training and TFS have different latest versions. | |||
Otherwise, this would simply be a single latest version. | |||
""" | |||
if Version(tensorflow_training_latest_version) in SpecifierSet(">=2.16"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: you can prob just assign a variable for the version object so you don't have to reinstantiate multiple times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, made the change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue #, if available:
Description of changes:
TF-2.16 test modification and handling: For TF2.16 DLCs, we have TF2.16.2 training and TF2.16.1 inference images. The counterparts for both of them do not exist which is by-default assumed by the test suite. This causes failures.
TF images do not have a 1:1 connection between training and TF-Serving versions anymore, thus, we have to handle this for TF2.16 images.
To handle this, in Tensorflow Estimator, we just use themajor.minor
version for TF 2.16.* images when retrieving theimage_uri
.This is being handled on the test side by making the
tf_full_version
point toTF2.16
to ensure tests that usefit
anddeploy
using the same estimator do not fail.Testing done:
CICD
Merge Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_base
to create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.