IFC-1940: Add metadata to graphql response #7683
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #7683 will not alter performanceComparing Summary
|
|
|
||
| async def _build_meta_response(self, db: InfrahubDatabase, field_name: str, fields: dict) -> dict: | ||
| data = {} | ||
| for meta_field in fields.get(field_name).keys(): |
There was a problem hiding this comment.
This looks like it could potentially be problematic if fields.get(field_name) were to return None and we then call .keys() on None. It might be that within this context it should never happen. But in the function signature we don't have any type of guarantee that this would be the case. Is there something we can change around this?
| data[meta_field] = peer.id if peer else None | ||
| continue | ||
|
|
||
| if meta_field_data is not None and isinstance(meta_field_data, str): |
There was a problem hiding this comment.
We can skip both of the meta_field_data is not None checks here as we're already checking the variable with isinstance. I.e. if it's a string or a RelationshipManager it will never be None.
|
|
||
| class BaseAttribute(ObjectType): | ||
| class InfrahubAttributeMetaObject(ObjectType): | ||
| updated_by = String(required=False, description="UUID of the user that last modified the attribute or relationship") |
There was a problem hiding this comment.
Small comment here regarding the UUID. I think that Aaron had a hardcoded user with the name -system- or similar. Not sure if we want to mention something like that in these descriptions? As it's just a description and not a strict type other systems wouldn't expect there to always be a UUID behind this response.
This is more of an observation, and I'm not sure about the correct approach if we should just leave it as it is now.
No description provided.