Conversation
syandroo
requested review from
ivan-khomyakov-sc,
phil-scale,
sasha-scale,
seanshi-scale and
yixu34
nucleus/model.py
Outdated
| name=payload["name"], | ||
| reference_id=payload["ref_id"], | ||
| metadata=payload["metadata"] or None, | ||
| bundle_id=payload["bundle_id"] or None, |
There was a problem hiding this comment.
I am curious, is payload["smth"] or None is justified, or payload.get("smth", None) would be better? Or wee need the code to break if bundle_id is not provided?
There was a problem hiding this comment.
hmm - interesting question, i think metadata is also not a required field but the thing that calls this defaults it to None also so this is really just a fallback
There was a problem hiding this comment.
+1 for payload.get("smth", None)
|
The interface change looks reasonable to me, though I wonder if you want to use |
|
@phil-scale good q, it's actually mostly semantic since what we actually store is the bundle_name, (we used to refer to the bundle_name as bundle_id then moved some to bundle_name). on the user side we never expose the uuid that bundle or endpoint has, just their names. |
|
i just wanted both the client and the API endpoint to have the same name, but i can change both to bundle_name if u think it's needed, just would require a lil bit more coordination on deploying. |
|
I agree with Phil, let's keep the user interface always using user defined names |
nucleus/model.py
Outdated
| """ | ||
|
|
||
| def __init__(self, model_id, name, reference_id, metadata, client): | ||
| def __init__(self, model_id, name, reference_id, metadata, bundle_id, client): |
There was a problem hiding this comment.
Should we have a default bundle_id=None or is that something that Nucleus doesn't do? I guess if this isn't ever user-created it's probably fine to not have this default?
syandroo
left a comment
syandroo
left a comment
There was a problem hiding this comment.
need to commit from phone
|
Don't we want to base this off of the |
This looks like client changes on the nucleus side to support Launch integration, so I think this goes on |
|
that's correct, OK to merge this into master (independent of HMI release branch) |
seanshi-scale
left a comment
seanshi-scale
left a comment
There was a problem hiding this comment.
I'll defer to someone on nucleus but this looks good to me
6 participants
added the ability to link a bundle to the model project object in #34891, this adds it to the client so we can demo