-
Notifications
You must be signed in to change notification settings - Fork 4.2k
refactor(connector): update add connector script with new connector features #8213
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
Conversation
Changed Files
|
e2162f9
to
b9aa52c
Compare
b9aa52c
to
0095cc5
Compare
8d820fc
to
0195f2b
Compare
0195f2b
to
5f8ceb1
Compare
for file in "${default_impl_files[@]}"; do | ||
tmpfile="${file}.tmp" | ||
|
||
awk -v prev="$previous_connector_camelcase" -v new="$payment_gateway_camelcase" ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we at least have some comments as to what is happening here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, in default_implementations.rs
,default_implementations_v2.rs
, the new connector was always added at the top of the macro list, without maintaining alphabetical order.
Also using a single sed
command to insert the new connector depends on the presence of the previous_connector
in the macro block. This approach fails when the previous_connector
is missing from one or more lists, leading to skipped insertions.
To handle this correctly, the current logic does the following:
- If the
previous_connector
exists in the macro, thenew_connector
is added after it, preserving alphabetical order. - If the
previous_connector
is not found, thenew_connector
is inserted at the top of the macro list.
I've also added comments in the code.
|
||
|
||
# Remove temporary files created in above step | ||
rm $conn.rs-e $src/types/api.rs-e $src/configs/settings.rs-e config/development.toml-e config/docker_compose.toml-e config/config.example.toml-e loadtest/config/development.toml-e crates/api_models/src/connector_enums.rs-e crates/euclid/src/enums.rs-e crates/api_models/src/routing.rs-e $src/core/payments/flows.rs-e crates/common_enums/src/connector_enums.rs-e $src/types/transformers.rs-e $src/core/admin.rs-e crates/hyperswitch_connectors/src/default_implementations.rs-e crates/hyperswitch_connectors/src/default_implementations_v2.rs-e crates/hyperswitch_interfaces/src/configs.rs-e $src/connector.rs-e config/deployments/integration_test.toml-e config/deployments/production.toml-e config/deployments/sandbox.toml-e temp crates/connector_configs/src/connector.rs-e crates/router/tests/connectors/main.rs-e crates/router/src/core/payments/connector_integration_v2_impls.rs-e | ||
|
||
sed -i'' -e "/pub ${previous_connector}: ConnectorParams,/a\\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this going to generate more temporary files, but we' have a step before this which says "Remove temporary files created in above step".
Can we move this line above the rm
command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’re already running the rm
command immediately after the sed edit for crates/hyperswitch_domain_models/src/configs.rs
, so the temp file (.rs-e) is cleaned up right away. I’ve tested the script multiple times, and no temporary files are left behind.
sed -i'' -e "/pub ${previous_connector}: ConnectorParams,/a\\
pub ${payment_gateway}: ConnectorParams,
" crates/hyperswitch_domain_models/src/configs.rs
rm crates/hyperswitch_domain_models/src/configs.rs-e
What are your thoughts? Do we still need to move this code above, or is the current placement okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we then group all the sed
commands together, followed by one rm
command? So move this sed
command before the big rm
command?
7a8c0ad
f5190dd
Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
This PR only updates the script to generate template code for a connector, hence no API testing required.
Script used to test the template generation.
sh scripts/add_connector.sh testConnector https://www.google.com/
Checklist
cargo +nightly fmt --all
cargo clippy