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

feat(columbus): add "service" field to columbus payload #264

Merged
merged 7 commits into from
Nov 4, 2021

Conversation

StewartJingga
Copy link
Contributor

No description provided.

@StewartJingga StewartJingga added the sink Add new sink label Oct 29, 2021
@StewartJingga StewartJingga added this to the v0.2 milestone Oct 29, 2021
@StewartJingga StewartJingga self-assigned this Oct 29, 2021
@StewartJingga StewartJingga marked this pull request as ready for review November 2, 2021 03:51
@@ -9,4 +9,5 @@ import (
type Metadata interface {
GetResource() *common.Resource
GetProperties() *facets.Properties
GetLineage() *facets.Lineage
Copy link
Member

Choose a reason for hiding this comment

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

@StewartJingga This would not be applicable to every resource right? Like user.

Copy link
Contributor Author

@StewartJingga StewartJingga Nov 4, 2021

Choose a reason for hiding this comment

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

yes, but right now it is the only way Columbus sink is able to fetch lineage from our varies models.

The other way is to typechecking for each types which im not sure is scalable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ravisuhag another approach is to have another interface for lineage and typecheck for that interface in Columbus.

type HasLineage interface {
  GetLineage() *facets.Lineage
}

Copy link
Member

Choose a reason for hiding this comment

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

I think let's create another interface for lineage check instead of expecting all models to have it.

Copy link
Member

@ravisuhag ravisuhag left a comment

Choose a reason for hiding this comment

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

LGTM

@ravisuhag ravisuhag merged commit f5b9938 into main Nov 4, 2021
@ravisuhag ravisuhag deleted the columbus-sink-send-service branch November 4, 2021 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sink Add new sink
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants