Skip to content
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

Correct event relationships #121

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

mikemrm
Copy link
Contributor

@mikemrm mikemrm commented Jun 21, 2023

We were improperly building relationships setting the relation to the type of resource rather than the actual relation it had.

This now finds all the relations which are supported by the two resources and creates the appropriate relationships.

Signed-off-by: Mike Mason <[email protected]>
Signed-off-by: Mike Mason <[email protected]>
We need to dig into the errors that may be produced and create a list of
errors which should be reprocessable.

Otherwise we might reprocess a message which will never be succeed,
wasting resources.

Signed-off-by: Mike Mason <[email protected]>
@mikemrm mikemrm force-pushed the correct-event-relationships branch 2 times, most recently from b748030 to ded2de5 Compare June 21, 2023 22:22
@mikemrm mikemrm marked this pull request as ready for review June 21, 2023 22:31
@mikemrm mikemrm requested review from a team as code owners June 21, 2023 22:31
fishnix
fishnix previously approved these changes Jun 21, 2023
Copy link
Contributor

@fishnix fishnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good except for the one comment - are we taking into account the union types?

Comment on lines 177 to 181
rType := s.qe.GetResourceType(resource.Type)
if rType == nil {
s.logger.Warnw("no resource type found for", "resource_type", resource.Type)

return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just realized you can do this outside of the loop right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, good point. fixing

This allows for relations to be determined based on their type.

All matched relations will be created.

Signed-off-by: Mike Mason <[email protected]>
@mikemrm mikemrm merged commit 1631f87 into infratographer:main Jun 21, 2023
@mikemrm mikemrm deleted the correct-event-relationships branch June 21, 2023 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants