Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[flyte deck] Streaming Decks #2779
[flyte deck] Streaming Decks #2779
Changes from 70 commits
01182b4
9616fc3
98ae7c7
4e92bb0
4df18b5
ebb4d4e
99522d9
c19d67d
67cd829
06da3df
6d99d69
b805cd7
4c97758
7b3574a
9b60564
18c994f
39f39d1
aabcbbb
9ca43f3
ed56352
b559fc9
fc5578f
7139468
e0aee9e
3727588
ce3ee15
d066231
f9387ce
f14c3fa
c33a909
8666c60
93580d6
bcaaabd
a321700
6464fae
884943c
d4b5b96
406227c
1e77f54
d70a2d5
7fc6393
6980140
c87a342
f609760
473ae11
008fe52
f0b9028
d48efa9
6b55930
b5912fb
a681ccd
048fdff
7bcf15e
b6c41c3
be02f9f
cf83e06
e137328
a59a56e
b8383be
dc6d203
41d8760
b71cc19
b5976fe
0c1a5a3
d082456
4a8c68f
b58527b
2764ed4
90372db
d8c408c
5cacf11
c447793
c5cc967
1d6417a
b0cd1ae
974b882
ea8b6e0
a642c9d
e9aef35
04775d7
34a4146
f30382b
b649f8f
0565282
c0fd23a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
nice.
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.
Can
ENABLE_DECK
be a in aconstants.py
file?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.
I think referring
Deck.publish()
in_output_deck
is strange. Can we have this error check be inDeck.publish()
?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.
yes it makes sense, thank you thomas <3
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.
Consider validating the
generates_deck
field before accessing it directly frompb2_object
. Similar to howinterruptible
is handled withHasField()
check.Code suggestion
Code Review Run #077cc1
Is this a valid issue, or was it incorrectly flagged by the Agent?
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.
Consider consolidating the
deck_settings
andno_deck_settings
into a single variable since they have identical configuration. This would reduce code duplication.Code suggestion
Code Review Run #077cc1
Is this a valid issue, or was it incorrectly flagged by the Agent?
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.
Consider consolidating the test cases by using
@pytest.mark.parametrize
to test bothenable_deck=True
andenable_deck=False
scenarios. This would make the test more maintainable and reduce code duplication.Code suggestion
Code Review Run #077cc1
Is this a valid issue, or was it incorrectly flagged by the Agent?