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

Refactor input/output types #232

Merged
merged 51 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
3188799
refactor: set defaults to topic types
sujuka99 Jun 2, 2023
be863ef
feat: Allow empty topic definitions, infer topics type
sujuka99 Jun 9, 2023
3a14350
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 12, 2023
56dc07b
chore: polish and clean up
sujuka99 Jun 12, 2023
13802de
chore: update poetry version in CI
sujuka99 Jun 12, 2023
b9cd890
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 13, 2023
0bd4736
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 13, 2023
11cb62a
chore: update schema
sujuka99 Jun 13, 2023
a514af7
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 13, 2023
43cfcf2
chore: pass tests
sujuka99 Jun 13, 2023
5b681c9
docs: update
sujuka99 Jun 13, 2023
1e64a6f
feat: enable type inference for components
sujuka99 Jun 13, 2023
5b860b6
docs(examples): Update to reflect the introduced feature
sujuka99 Jun 14, 2023
c3d9632
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 14, 2023
16468cb
style: update docstr
sujuka99 Jun 14, 2023
53663a1
misc: feedback
sujuka99 Jun 14, 2023
ee17c5f
refactor: disallow empty topic def
sujuka99 Jun 14, 2023
5a79f56
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 14, 2023
2566df6
tests: update to reflect extra options
sujuka99 Jun 14, 2023
96642c1
refactor: dont reassign type
sujuka99 Jun 14, 2023
10c283d
refactor: replace if-else with pattern-matching
sujuka99 Jun 14, 2023
e98161c
docs: update
sujuka99 Jun 14, 2023
35aa35a
docs: fix role and schema comments
sujuka99 Jun 14, 2023
4e51e5a
docs: fix extra-pattern-topic in examples
sujuka99 Jun 14, 2023
b61fdf9
docs: fix input-component name
sujuka99 Jun 14, 2023
06a1558
docs: improve comments in code
sujuka99 Jun 14, 2023
c251fe1
chore: pass tests
sujuka99 Jun 14, 2023
0c3dafc
refactor: add type assignment to root validator
sujuka99 Jun 14, 2023
7220ffd
refactor(from): simplify
sujuka99 Jun 15, 2023
21b728e
feat: remove redundant types from `from`
sujuka99 Jun 15, 2023
2d8f7fb
refactor: remove redundant output topic types
sujuka99 Jun 15, 2023
138faae
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 15, 2023
a05beff
chore: pass tests
sujuka99 Jun 15, 2023
e4695a4
docs(examples): Update
sujuka99 Jun 15, 2023
dfe9d8f
style: remove type from docstr
sujuka99 Jun 15, 2023
d3f9bb1
refactor: move type implication into component
sujuka99 Jun 27, 2023
8bb1a19
fix: topic weaving
sujuka99 Jun 27, 2023
a9d5c18
chore: pass tests
sujuka99 Jun 27, 2023
0d175c0
Merge remote-tracking branch 'origin/main' into refactor/input-output…
sujuka99 Jun 27, 2023
19bc197
Merge remote-tracking branch 'origin/v2' into refactor/input-output-t…
sujuka99 Aug 15, 2023
8af5212
docs: update from and to sections
sujuka99 Aug 15, 2023
efec342
fix: revert wrong change
sujuka99 Aug 15, 2023
2e071c1
chore: generate docs
sujuka99 Aug 15, 2023
ca33389
chore: revert problems from the merge
sujuka99 Aug 15, 2023
137f219
fix: tests
sujuka99 Aug 15, 2023
5ab0ad1
chore: trailing comma
sujuka99 Aug 15, 2023
bf57e19
fix: typo
sujuka99 Aug 15, 2023
0db3ea5
Apply suggestions from code review
sujuka99 Aug 15, 2023
32bc451
fix: missing import
sujuka99 Aug 15, 2023
f180073
docs: Do not document usage of `null`
sujuka99 Aug 16, 2023
e7ca9ac
docs: fix typo
sujuka99 Aug 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions docs/docs/schema/pipeline.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"description": "Input topic",
"properties": {
"role": {
"description": "Custom identifier belonging to a topic, provide only if `type` is `extra` or `extra-pattern`",
"description": "Custom identifier belonging to a topic, provide only if `type` is `extra`, `pattern` or `extra-pattern`",
disrupted marked this conversation as resolved.
Show resolved Hide resolved
"title": "Role",
"type": "string"
},
Expand All @@ -41,12 +41,10 @@
"$ref": "#/definitions/InputTopicTypes"
}
],
"default": "input",
"description": "Topic type"
}
},
"required": [
"type"
],
"title": "FromTopic",
"type": "object"
},
Expand Down Expand Up @@ -91,6 +89,7 @@
"enum": [
"input",
"extra",
"pattern",
Copy link
Member

Choose a reason for hiding this comment

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

Why is there still extra? And why is there pattern and input-pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise there would be breaking changes

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 that would be fine, user base is small. What do you think @disrupted @raminqaf ?

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise we keep carrying technical debt because decision in the beginning weren't ideal

Copy link
Member

Choose a reason for hiding this comment

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

I am fine with it, but keep in mind we would have to do a major version bump for the next release, aka kpops 2.0

Copy link
Member

Choose a reason for hiding this comment

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

yeah, but we're not pre-release (0.x.x) anymore. Since we use semantic versioning we should follow its rules.

Copy link
Member

Choose a reason for hiding this comment

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

I mean it is also fine to have v2 and quite soon v3

Copy link
Member

Choose a reason for hiding this comment

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

yep, absolutely!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Will carry out the breaking changes in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philipp94831 I think I'm done with the changes to the code, what do you think?

"input-pattern",
"extra-pattern"
],
Expand Down Expand Up @@ -1225,6 +1224,7 @@
"$ref": "#/definitions/OutputTopicTypes"
}
],
"default": "output",
"description": "Topic type"
},
"valueSchema": {
Expand All @@ -1233,9 +1233,6 @@
"type": "string"
}
},
"required": [
"type"
],
"title": "TopicConfig",
"type": "object"
}
Expand Down
8 changes: 6 additions & 2 deletions kpops/components/base_components/models/from_section.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class InputTopicTypes(str, Enum):

INPUT = "input"
EXTRA = "extra"
PATTERN = "pattern"
INPUT_PATTERN = "input-pattern"
EXTRA_PATTERN = "extra-pattern"

Expand All @@ -25,11 +26,13 @@ class FromTopic(BaseModel):

:param type: Topic type
:type type: InputTopicTypes
:param role: Custom identifier belonging to a topic, provide only if `type` is `extra` or `extra-pattern`
:param role: Custom identifier belonging to a topic, provide only if `type` is `extra`, `pattern` or `extra-pattern`
:type role: str | None
"""

type: InputTopicTypes = Field(..., description=describe_attr("type", __doc__))
type: InputTopicTypes = Field(
default=InputTopicTypes.INPUT, description=describe_attr("type", __doc__)
)
role: str | None = Field(default=None, description=describe_attr("role", __doc__))

class Config(DescConfig):
Expand All @@ -41,6 +44,7 @@ def extra_topic_role(cls, values: dict) -> dict:
"""Ensure that cls.role is used correctly"""
is_extra_topic = values["type"] in (
InputTopicTypes.EXTRA,
InputTopicTypes.PATTERN,
InputTopicTypes.EXTRA_PATTERN,
)
if is_extra_topic and not values.get("role"):
Expand Down
4 changes: 3 additions & 1 deletion kpops/components/base_components/models/to_section.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ class TopicConfig(BaseModel):
:type role: str | None
"""

type: OutputTopicTypes = Field(..., description="Topic type")
type: OutputTopicTypes = Field(
default=OutputTopicTypes.OUTPUT, description="Topic type"
)
key_schema: str | None = Field(
default=None, alias="keySchema", description="Key schema class name"
)
Expand Down
2 changes: 1 addition & 1 deletion kpops/components/base_components/pipeline_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class PipelineComponent(BaseDefaultsComponent):
description=describe_object(__doc__),
exclude=True,
)
name: str = Field(default=..., description="Component name")
name: str = Field(default=..., description=describe_attr("name", __doc__))
from_: FromSection | None = Field(
default=None,
alias="from",
Expand Down
11 changes: 4 additions & 7 deletions tests/cli/snapshots/snap_test_schema_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"description": "Input topic",
"properties": {
"role": {
"description": "Custom identifier belonging to a topic, provide only if `type` is `extra` or `extra-pattern`",
"description": "Custom identifier belonging to a topic, provide only if `type` is `extra`, `pattern` or `extra-pattern`",
"title": "Role",
"type": "string"
},
Expand All @@ -108,12 +108,10 @@
"$ref": "#/definitions/InputTopicTypes"
}
],
"default": "input",
"description": "Topic type"
}
},
"required": [
"type"
],
"title": "FromTopic",
"type": "object"
},
Expand All @@ -122,6 +120,7 @@
"enum": [
"input",
"extra",
"pattern",
"input-pattern",
"extra-pattern"
],
Expand Down Expand Up @@ -379,6 +378,7 @@
"$ref": "#/definitions/OutputTopicTypes"
}
],
"default": "output",
"description": "Topic type"
},
"valueSchema": {
Expand All @@ -387,9 +387,6 @@
"type": "string"
}
},
"required": [
"type"
],
"title": "TopicConfig",
"type": "object"
}
Expand Down