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: URL Parameters #115

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: URL Parameters #115

wants to merge 8 commits into from

Conversation

Day-OS
Copy link

@Day-OS Day-OS commented Mar 27, 2024

(FROM README)

URL Parameter Configuration

Option default type description
record_key - String A key from a JSON record that will be used to insert a value to the parameter
url_key - String Parameter's key name
prefix - String String to be added before the value
suffix - String String to be added after the value
Example: Inserting an ID from a JSON record into a URL Parameter

Let's assume a scenario that the following endpoint requires a SQL condition to update a piece of information: https://someurl.com/

It accepts updatecondition=<SQL CONDITION HERE> as a parameter to set the condition for the exact row that we want to update.

We have the following JSON record that we want to get updated:

  {
    "id": 2901,
    "name": "Luiz Barros Rocha",
    "age": 26
  }

We could write a url_parameter containing the following information:

  {
    url_parameters: 
    - url_key: updatecondition
      record_key: id
      prefix: "user_id = "
  }

This would be the ending result:

https://someurl.com?updatecondition=user_id+%3D+2901

@Day-OS Day-OS changed the title URL Parameters feat: URL Parameters Mar 27, 2024
@digikata digikata self-requested a review March 27, 2024 15:35
@sehz
Copy link
Contributor

sehz commented Mar 27, 2024

Good suggestion. However, content transformation use can be infinite. Instead, intent is to have content to be processed using transformation approach like here: https://infinyon.com/blog/2023/09/github-stars-to-slack.

So approach is create SmartModule to process url and send out appropriate transformation

@digikata
Copy link
Contributor

@Day-OS if you cherry pick the commit on https://github.com/infinyon/http-sink-connector/tree/ac/params-reuse-request I refactored so the client and most of the request is not reinitialized each loop.

(I think there was also a little buglet fixed in this refactor where the loop was sending the current record to the previous records URL with query parameters)

@Day-OS
Copy link
Author

Day-OS commented Mar 28, 2024

@Day-OS if you cherry pick the commit on https://github.com/infinyon/http-sink-connector/tree/ac/params-reuse-request I refactored so the client and most of the request is not reinitialized each loop.

(I think there was also a little buglet fixed in this refactor where the loop was sending the current record to the previous records URL with query parameters)

It looks good, I've cherry-picked it

Copy link
Contributor

@digikata digikata left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Day-OS

}

#[async_trait]
impl Sink<String> for HttpSink {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the Sink trait impl was removed? This is a contract that all sink connectors must follow.


#[derive(Debug)]
pub(crate) struct HttpSink {
#[allow(dead_code)]
client: Client,
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is not used anywhere, why do we need?

@@ -32,6 +33,19 @@ pub(crate) struct HttpConfig {
/// Http connect timeout in milliseconds
#[serde(with = "humantime_serde", default = "default_http_connect_timeout")]
pub http_connect_timeout: Duration,

//HTTP Parameters that can be gattered from a Message if the message is a json file
#[serde(default = "default_http_params")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[serde(default = "default_http_params")]
#[serde(default)]

if let Some(ref suffix) = param.suffix {
value = value.clone() + &suffix;
}
builder = builder.query(&[(encode(&url_key), &value)]);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use references at the end, can we do the same without all those clonings above?

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.

4 participants