-
Notifications
You must be signed in to change notification settings - Fork 1
Add stategic metadata to insert_submission, SubmissionCompleted and SubmissionFailed #58
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
base: master
Are you sure you want to change the base?
Conversation
Qqwy
left a comment
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.
Hey Jeremy! A very nice PR so far. The one request I have is that you add a test for both of the cases (to the Python test suite).
aa4f7ba to
d5db03b
Compare
42a9a61 to
8486574
Compare
8486574 to
ac3e448
Compare
| serialization_format: SerializationFormat = DEFAULT_SERIALIZATION_FORMAT, | ||
| metadata: None | bytes = None, | ||
| strategic_metadata: None | dict[str, str | int] = None, | ||
| strategic_metadata: None | dict[str, int] = None, |
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.
These were adjusted during testing, since attempting to pass something of type dict[str, str] produced the following run-time type error TypeError: argument 'strategic_metadata': 'str' object cannot be interpreted as an integer, which seems to stem from the underlying rust definition of strategic metadata which does not support strings as values:
opsqueue/opsqueue/src/common/mod.rs
Line 14 in e823308
| pub type StrategicMetadataMap = FxHashMap<String, MetaStateVal>; |
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's indeed the case that currently only ints are supported.
If it still gives you problems, I guess the actual appropriate Python type to use here would be Mapping[str, int] (the immutable version of dict), which matters because of variance.
Add stategic metadata as a parameter to
insert_submission(which just passes it toinsert_submission_chunks), and make the strategic metadata available in the submission results:SubmissionCompletedandSubmissionFailed.