Conversation
| @@ -0,0 +1,431 @@ | |||
| #!/usr/bin/env python3 | |||
| """Protobuf Version-Skew Tester for Flyte | |||
There was a problem hiding this comment.
This is purely a proof-of-concept. Most of this code is bad, but conveys functionally how an automated version-skew test might work.
It assumes (I think this is the right assumption) that the backend (flyte) is using exactly the Golang flyteidl protobuf bindings, while the client (flytekit) is using exactly the Python flyteidl protobuf bindings. (But one possible improvement to this test could be to extend this assumption, if it is inaccurate in some ways.)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7147 +/- ##
==========================================
- Coverage 56.95% 56.95% -0.01%
==========================================
Files 931 931
Lines 58188 58188
==========================================
- Hits 33141 33140 -1
- Misses 22005 22006 +1
Partials 3042 3042
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -0,0 +1,431 @@ | |||
| #!/usr/bin/env python3 | |||
| """Protobuf Version-Skew Tester for Flyte | |||
There was a problem hiding this comment.
Incompatible protobuf change example - testing master...ddl-rliu:flyte:rliu.protos-test
$ .venv/bin/python test_proto_version_skew.py --head rliu.protos-test
Analyzed branch ddl-rliu/flyte:rliu.protos-test
Detected changes to: OutputReference
OutputReference (flyteidl/core/types.proto)
~ var: string -> int32
Building Go deserialization helpers...
Testing version-skew
Backend (flyte) and Client (flytekit)
- Old: master@558cab64ffb60044653f19f1ea952133e233208e
- New: rliu.protos-test@d1c0a6d29e81f6db1d0f7295bea1639e667811d1
Test 1: New client (flytekit) and old backend (flyte)
message = OutputReference(
node_id="test",
var=1,
attr_path=[],
)
New client (flytekit) serializes the message to: 0a04746573741001
Old backend (flyte) deserializes the message to: OutputReference("test", "", &{{0x14000108a80 <nil> 22} 0x1400002c690})
Test 2: Old client (flytekit) and new backend (flyte)
message = OutputReference(
node_id="test",
var="test",
attr_path=[],
)
Old client (flytekit) serializes the message to: 0a0474657374120474657374
New backend (flyte) deserializes the message to: OutputReference("test", 0, &{{0x140000b8a80 <nil> 22} 0x140000aa650})
Version-skew compatibility: PASS (but please manually verify the results)
Manually verify the results - notice that Old backend (flyte) deserializes the message to: OutputReference("test", "", &{{0x14000108a80 <nil> 22} 0x1400002c690}) -> The second param should be 1.
And New backend (flyte) deserializes the message to: OutputReference("test", 0, &{{0x140000b8a80 <nil> 22} 0x140000aa650}) -> The second param should be "test"
-> Fail
ddl-rliu
force-pushed
the
rliu.version-skew-test
branch
from
e61ad1d to
d85f94e
Compare
ddl-rliu
force-pushed
the
rliu.version-skew-test
branch
from
d85f94e to
0eec2b4
Compare
| @@ -0,0 +1,431 @@ | |||
| #!/usr/bin/env python3 | |||
| """Protobuf Version-Skew Tester for Flyte | |||
There was a problem hiding this comment.
File extension PR - testing https://github.com/ddl-rliu/flyte/tree/rliu.DOM-75010.file-ext-copilot
$ .venv/bin/python test_proto_version_skew.py --head rliu.DOM-75010.file-ext-copilot
Analyzed branch ddl-rliu/flyte:rliu.DOM-75010.file-ext-copilot
Detected changes to: BlobType
BlobType (flyteidl/core/types.proto)
+ string file_extension
+ bool enable_legacy_filename
Building Go deserialization helpers...
Testing version-skew
Backend (flyte) and Client (flytekit)
- Old: master@558cab64ffb60044653f19f1ea952133e233208e
- New: rliu.DOM-75010.file-ext-copilot@9327eb02af6acac3d31c844413583cd9d13a74d0
Test 1: New client (flytekit) and old backend (flyte)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
)
New client (flytekit) serializes the message to: 0a0474657374
Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
file_extension="test",
enable_legacy_filename=True,
)
New client (flytekit) serializes the message to: 0a04746573741a04746573742001
Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE)
Test 2: Old client (flytekit) and new backend (flyte)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
)
Old client (flytekit) serializes the message to: 0a0474657374
New backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE, "", False)
Version-skew compatibility: PASS (but please manually verify the results)
Manually verify the results - notice that Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE) - the optional params are dropped, which is the desired behavior.
And New backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE, "", False) - the optional params are populated with the default values "", False, which is the desired behavior.
There was a problem hiding this comment.
Testing again with new changes
.venv/bin/python test_proto_version_skew.py --head rliu.DOM-75010.file-ext-copilot
Analyzed branch ddl-rliu/flyte:rliu.DOM-75010.file-ext-copilot
Detected changes to: BlobType
BlobType (flyteidl/core/types.proto)
+ string file_extension
Building Go deserialization helpers...
Testing version-skew
Backend (flyte) and Client (flytekit)
- Old: master@558cab64ffb60044653f19f1ea952133e233208e
- New: rliu.DOM-75010.file-ext-copilot@7c7a6327827c50eba4ce1c5cc007012ff553a7bc
Test 1: New client (flytekit) and old backend (flyte)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
)
New client (flytekit) serializes the message to: 0a0474657374
Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
file_extension="test",
)
New client (flytekit) serializes the message to: 0a04746573741a0474657374
Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE)
Test 2: Old client (flytekit) and new backend (flyte)
message = BlobType(
format="test",
dimensionality=BlobType.SINGLE,
)
Old client (flytekit) serializes the message to: 0a0474657374
New backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE, "")
Version-skew compatibility: PASS (but please manually verify the results)
Manually verify the results - notice that Old backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE) - the optional params are dropped, which is the desired behavior.
And New backend (flyte) deserializes the message to: BlobType("test", BlobType.SINGLE, "") - the optional file_extension is populated with the default value "", which is the desired behavior.
1 participant
Tracking issue
Why are the changes needed?
What changes were proposed in this pull request?
How was this patch tested?
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link