-
Notifications
You must be signed in to change notification settings - Fork 301
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
Signal use #1398
Merged
Merged
Signal use #1398
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
15bd462
start pr for testing
wild-endeavor db3b468
need to switch
wild-endeavor 1247e65
merge master and resolve
wild-endeavor d73134e
remove
wild-endeavor a7ff035
mid-adding to flyte remote
wild-endeavor 2ec9a6f
wrong use of model files
wild-endeavor 708ad04
nit
wild-endeavor a6a1698
add a couple tests
wild-endeavor 793f42b
Merge remote-tracking branch 'origin/master' into signal-use
wild-endeavor 82ad400
fix some tests
wild-endeavor 6b96edb
lint
wild-endeavor 12eb65b
Update flytekit/remote/entities.py
wild-endeavor 22b2544
Update flytekit/clients/raw.py
wild-endeavor 254a3ea
rename test
wild-endeavor 291128c
merge and raise back to info
wild-endeavor 65b7e62
Merge branch 'master' into signal-use
wild-endeavor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
from flyteidl.admin.signal_pb2 import Signal, SignalList | ||
from mock import MagicMock | ||
|
||
from flytekit.configuration import Config | ||
from flytekit.core.context_manager import FlyteContextManager | ||
from flytekit.core.type_engine import TypeEngine | ||
from flytekit.models.core.identifier import SignalIdentifier, WorkflowExecutionIdentifier | ||
from flytekit.remote.remote import FlyteRemote | ||
|
||
|
||
def test_remote_list_signals(): | ||
ctx = FlyteContextManager.current_context() | ||
wfeid = WorkflowExecutionIdentifier("p", "d", "execid") | ||
signal_id = SignalIdentifier(signal_id="sigid", execution_id=wfeid).to_flyte_idl() | ||
lt = TypeEngine.to_literal_type(int) | ||
signal = Signal( | ||
id=signal_id, | ||
type=lt.to_flyte_idl(), | ||
value=TypeEngine.to_literal(ctx, 3, int, lt).to_flyte_idl(), | ||
) | ||
|
||
mock_client = MagicMock() | ||
mock_client.list_signals.return_value = SignalList(signals=[signal], token="") | ||
|
||
remote = FlyteRemote(config=Config.auto(), default_project="p1", default_domain="d1") | ||
remote._client = mock_client | ||
res = remote.list_signals("execid", "p", "d", limit=10) | ||
assert len(res) == 1 | ||
|
||
|
||
def test_remote_set_signal(): | ||
mock_client = MagicMock() | ||
|
||
def checker(request): | ||
assert request.id.signal_id == "sigid" | ||
assert request.value.scalar.primitive.integer == 3 | ||
|
||
mock_client.set_signal.side_effect = checker | ||
|
||
remote = FlyteRemote(config=Config.auto(), default_project="p1", default_domain="d1") | ||
remote._client = mock_client | ||
remote.set_signal("sigid", "execid", 3) |
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.
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.
qq: should the name of the gate node be unique?
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.
so this is actually an interesting question. signals ids are unique within a workflow execution. so if you had this same statement twice it would just wait twice for a single signal with the given name. nothing would break unless the expected types are different. presumably you could have two subworkflows that both wait on the same signal by using the same name. at least that's how it works in the backend. not sure if we want to explicitely restrict this from flytekit side?
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.
got it, thanks for the explain
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.