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

Generate schema in CI #197

Merged
merged 25 commits into from
Apr 27, 2023
Merged

Generate schema in CI #197

merged 25 commits into from
Apr 27, 2023

Conversation

sujuka99
Copy link
Contributor

closes #71

Reasoning: Ease testing, provide more flexibility
Do not make snapshots of the real schemas, this is handled in the CI
Focus on making sure that the functions work as intended.

No need to test output correctness of`gen_config_schema()`, it is
essentially an alias for the pydantic schema_json_of() command
@sujuka99 sujuka99 self-assigned this Apr 21, 2023
@sujuka99 sujuka99 marked this pull request as ready for review April 21, 2023 13:55
.github/workflows/ci.yaml Show resolved Hide resolved
.github/scripts/run_mypy.sh Outdated Show resolved Hide resolved
docs/docs/user/references/cli-commands.md Show resolved Hide resolved
hooks/run_mypy.sh Outdated Show resolved Hide resolved
kpops/utils/gen_schema.py Outdated Show resolved Hide resolved
kpops/utils/gen_schema.py Outdated Show resolved Hide resolved
hooks/gen_schema.sh Outdated Show resolved Hide resolved
kpops/utils/gen_schema.py Show resolved Hide resolved
@disrupted
Copy link
Member

the gen-docs hook is failing on my local machine

gen-docs.................................................................Failed
- hook id: gen-docs
- exit code: 1
- files were modified by this hook

Docs saved to: /Users/disrupted/bakdata/kpops/docs/docs/user/references/cli-commands.md
sed: 1: "/Users/disrupted/bakdat ...": extra characters at the end of d command

might be due to different version of sed?!

tests/cli/test_schema_generation.py Outdated Show resolved Hide resolved
tests/cli/test_schema_generation.py Outdated Show resolved Hide resolved
@sujuka99
Copy link
Contributor Author

sujuka99 commented Apr 26, 2023

the gen-docs hook is failing on my local machine

gen-docs.................................................................Failed
- hook id: gen-docs
- exit code: 1
- files were modified by this hook

Docs saved to: /Users/disrupted/bakdata/kpops/docs/docs/user/references/cli-commands.md
sed: 1: "/Users/disrupted/bakdat ...": extra characters at the end of d command

might be due to different version of sed?!

Yea, it behaves differently on mac, I will fix it

I guess it didn't work at all until now if it never failed for you or @raminqaf (He also uses macOS, right?)

@disrupted
Copy link
Member

disrupted commented Apr 26, 2023

might be due to different version of sed?!

Yea, it behaves differently on mac, I will fix it

now I am getting

gen-docs.................................................................Failed
- hook id: gen-docs
- exit code: 1
- files were modified by this hook

Docs saved to: /Users/disrupted/bakdata/kpops/docs/docs/user/references/cli-commands.md
sed: illegal option -- -
usage: sed script [-Ealnru] [-i extension] [file ...]
        sed [-Ealnu] [-i extension] [-e script] ... [-f script_file] ... [file ...]

edit: I believe sed -i '' ... works

@sujuka99
Copy link
Contributor Author

might be due to different version of sed?!

Yea, it behaves differently on mac, I will fix it

now I am getting

gen-docs.................................................................Failed
- hook id: gen-docs
- exit code: 1
- files were modified by this hook

Docs saved to: /Users/disrupted/bakdata/kpops/docs/docs/user/references/cli-commands.md
sed: illegal option -- -
usage: sed script [-Ealnru] [-i extension] [file ...]
        sed [-Ealnu] [-i extension] [-e script] ... [-f script_file] ... [file ...]

edit: I believe sed -i '' ... works

How about now? Apparently there is no way to make the -i flag work on both Zsh and bash with the same syntax..

I added a check in the CI to ensure that the scripts work in Zsh in this PR

Copy link
Member

@disrupted disrupted left a comment

Choose a reason for hiding this comment

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

last one, otherwise LGTM

tests/cli/test_schema_generation.py Show resolved Hide resolved
@sujuka99 sujuka99 merged commit bf7ab44 into main Apr 27, 2023
@sujuka99 sujuka99 deleted the ci/schema branch April 27, 2023 11:45
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.

Generate schema in CI
2 participants