feat: include an optional list of tags when deploying a compute graph #1083
Conversation
miguelhrocha
added
ci_compat_test
server/data_model/src/lib.rs
Outdated
| @@ -345,6 +345,7 @@ pub struct ComputeGraph { | |||
| pub name: String, | |||
| pub description: String, | |||
| pub version: GraphVersion, // Version incremented with code update | |||
| pub tags: Vec<String>, | |||
There was a problem hiding this comment.
Can you check the behavior when there are no tags in storage?
I believe this will need #[serde(default)]
There was a problem hiding this comment.
We want all existing graphs to default to an empty Vec without erroring out.
There was a problem hiding this comment.
Please make sure:
- An older python-sdk can create and update graphs (it won't know about tags)
- Graphs existing on an older server can be fetched with a default of no tags
This is to make sure we don't require wiping storage or force everyone to upgrade their SDK version on deploy.
| ): | ||
| self.name = name | ||
| self.description = description | ||
| self.nodes: Dict[str, Union[IndexifyFunction, IndexifyRouter]] = {} | ||
| self.routers: Dict[str, List[str]] = defaultdict(list) | ||
| self.edges: Dict[str, List[str]] = defaultdict(list) | ||
| self.accumulator_zero_values: Dict[str, Any] = {} | ||
| self.tags = tags or [] |
There was a problem hiding this comment.
can we rename these to user tags or deployment tags?
There was a problem hiding this comment.
Can you describe why it would be preferable?
There was a problem hiding this comment.
I would like it to be very obvious what tags here is to avoid any confusion. Making the naming as obvious as possible helps.
There was a problem hiding this comment.
Maybe adding comments would help with removing any confusion.
There was a problem hiding this comment.
May I suggest creating a Dictionary, K -> V for these things? Name - Tags, Metadata or Labels are all fine.
| ): | ||
| self.name = name | ||
| self.description = description | ||
| self.nodes: Dict[str, Union[IndexifyFunction, IndexifyRouter]] = {} | ||
| self.routers: Dict[str, List[str]] = defaultdict(list) | ||
| self.edges: Dict[str, List[str]] = defaultdict(list) | ||
| self.accumulator_zero_values: Dict[str, Any] = {} | ||
| self.tags = tags or [] |
There was a problem hiding this comment.
May I suggest creating a Dictionary, K -> V for these things? Name - Tags, Metadata or Labels are all fine.
…ensorlakeai/indexify into miguelhrocha/add-tags-to-compute-graph
miguelhrocha
added
the
ci_compat_test
…ensorlakeai/indexify into miguelhrocha/add-tags-to-compute-graph
Context
This PR comes from a need for a visual aid to understand which compute graph is being currently deployed on Indexify.
This can be considered a short-term solution, as there is no constraint on creating graphs with the same tags collection. However, this can enable CI/CD to automatically generate tags based on commit SHAs.
What
Adds an optional list of tags which are displayed on the Indexify UI.
Bonus
Sorts the invocations of a compute graph by creation date because that was driving me crazy.
Contribution Checklist
make fmtinpython-sdk/.make fmtinserver/.