[composer] Update scripts generating connectors' config JSON schemas #5377
Conversation
Powlinett
requested review from
Kakudou,
Megafredo,
Ninoxe,
helene-nguyen,
jabesq,
mariot and
throuxel
Powlinett
added
filigran team
|
I don't link the issue to avoid closing it at merge (still opened for other PRs) |
Powlinett
force-pushed
the
fix/4857-update-config-json-schema-generation-scripts
branch
from
93e493e to
b54cfa3
Compare
jabesq
left a comment
jabesq
left a comment
There was a problem hiding this comment.
Mostly just some nitpicking. I've reviewed only the bash scripts because I've no knowledge with batch scripting
...d/tools/composer/generate_connectors_config_schemas/generate_connector_config_json_schema.sh
Outdated
Show resolved
Hide resolved
| echo -e "\033[32mFound pydantic-settings and/or connectors-sdk in requirements. Proceeding with schema generation...\033[0m" | ||
| # If not found in requirements.txt and pyproject.toml exists, try to find connectors-sdk in pyproject.toml | ||
| has_required_dependency=false | ||
| if grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then |
There was a problem hiding this comment.
suggestion: requirements.txt could not be present if all the requirements are defined in the pyproject.toml
| if grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then | |
| if [[ -n "$requirements_file" ]] &&grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then |
There was a problem hiding this comment.
For now, every connectors must have a requirements.txt to be compatible with our CI. Plus, the presence of requirements.txt is already checked on line 233.
...tools/composer/generate_connectors_config_schemas/generate_connectors_config_json_schemas.sh
Outdated
Show resolved
Hide resolved
| # Check if requirements file contains pydantic-settings or connectors-sdk dependency | ||
| # If not found in requirements.txt and pyproject.toml exists, try to find connectors-sdk in pyproject.toml | ||
| has_required_dependency=false | ||
| if grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then |
There was a problem hiding this comment.
suggestion: requirements.txt could not be present if all the requirements are defined in the pyproject.toml
| if grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then | |
| if [[ -n "$requirements_file" ]] && grep -qE 'pydantic-settings|connectors-sdk' "$requirements_file"; then |
There was a problem hiding this comment.
For now, every connectors must have a requirements.txt to be compatible with our CI.
| python -m pip install -qq -r "$requirements_file" | ||
|
|
||
| # Return to original working directory | ||
| cd "$original_dir" |
There was a problem hiding this comment.
suggestion: If you navigate within a single directory, you can use cd - to return to the previous folder without storing the path in a variable. If you navigate to multiple folders, you can use the pushd and popd commands.
Powlinett
force-pushed
the
fix/4857-update-config-json-schema-generation-scripts
branch
from
e0fdf89 to
88d68a0
Compare
Powlinett
added
the
do not merge
Powlinett
dismissed
jabesq’s stale review
I was wrong, some connectors don't have requirements.txt file, need to rework my fixes.
…ate_connector_config_json_schema.sh Co-authored-by: Hugo Dupras <[email protected]>
…ate_connectors_config_json_schemas.sh Co-authored-by: Hugo Dupras <[email protected]>
Powlinett
force-pushed
the
fix/4857-update-config-json-schema-generation-scripts
branch
from
4fa98ec to
fa9ff73
Compare
Powlinett
force-pushed
the
fix/4857-update-config-json-schema-generation-scripts
branch
from
fa9ff73 to
a72510f
Compare
Powlinett
removed
the
do not merge
💡 Note:
This PR is made to handle the generation of the config JSON schema for connectors like
dragos: #5374This PR should be merged first to avoid any issue with the CI
Proposed changes
pyproject.tomlin bash scriptspyproject.tomlin powershell scriptsRelated issues
Checklist
I added/update the relevant documentation (either on github or on notion)Where necessary I refactored code to improve the overall quality