ajvb / kala

Modern Job Scheduler
MIT License
2.11k stars 187 forks source link

[multipart] Support multipart/form request #229

Closed weburnit closed 4 years ago

weburnit commented 4 years ago

Dear friend,

I figured out that we can't configure multipart-form http.request therefore I refine a bit for this case. hope that helps

tooolbox commented 4 years ago

Thanks for the PR @weburnit

While I'm interested in Kala being able to send multipart form requests if this is what users need, I have some questions about the strategy and implementation in your PR.

Thanks!

tooolbox commented 4 years ago

Thanks @weburnit , I'm starting to understand a little more.

I have a question, can you simply do this:

data := map[string]string{
    "hello": "world",
    "foo": "young-padawan",
}
body := url.Values{}
for k, v := range data {
    body.Set(k, v)
}
j := &Job{
    Name:  "mock_job",
    Owner: "jedi@master.com",
    RemoteProperties: RemoteProperties{
        Url:  "http://" + srv.Listener.Addr().String() + "/path",
        Body: data.Encode(),
        Headers: map[string][]string{
            "Content-Type": {"application/x-www-form-urlencoded"},
        },
    },
}
r := JobRunner{
    job: j,
}
out, err := r.RemoteRun()

It seems like, since the body is any arbitrary string you want, you should be able to encode in whatever fashion before you give it to Kala. It doesn't have to be JSON. The above should work right now with the current version of the project.

By doing this, you should be able to accomplish what you want without modifying Kala. I think it's good to keep the project focused on one thing, rather than becoming a "kitchen sink".

I am not saying absolutely no but I want to just fully talk it over to make sure the right decisions are made.

Thanks!

weburnit commented 4 years ago

Actually, the body is gonna be encoded into JSON, later on, those are bind into Params. It's my honor for having your kind review!

weburnit commented 4 years ago

Thanks @weburnit , I'm starting to understand a little more.

I have a question, can you simply do this:

data := map[string]string{
    "hello": "world",
    "foo": "young-padawan",
}
body := url.Values{}
for k, v := range data {
    body.Set(k, v)
}
j := &Job{
  Name:  "mock_job",
  Owner: "jedi@master.com",
  RemoteProperties: RemoteProperties{
      Url:  "http://" + srv.Listener.Addr().String() + "/path",
      Body: data.Encode(),
      Headers: map[string][]string{
          "Content-Type": {"application/x-www-form-urlencoded"},
      },
  },
}
r := JobRunner{
  job: j,
}
out, err := r.RemoteRun()

It seems like, since the body is any arbitrary string you want, you should be able to encode in whatever fashion before you give it to Kala. It doesn't have to be JSON. The above should work right now with the current version of the project.

By doing this, you should be able to accomplish what you want without modifying Kala. I think it's good to keep the project focused on one thing, rather than becoming a "kitchen sink".

I am not saying absolutely no but I want to just fully talk it over to make sure the right decisions are made.

Thanks!

I've updated a bit! thanks for your review!

tooolbox commented 4 years ago

It's my honor for having your kind review!

No problem :)

So, your goal is to have Kala do a POST request with a form-encoded body, correct? Then this should work:

data := map[string]string{
    "hello": "world",
    "foo": "young-padawan",
}
body := url.Values{}
for k, v := range data {
    body.Set(k, v)
}
j := &Job{
    Name:  "mock_job",
    Owner: "jedi@master.com",
    RemoteProperties: RemoteProperties{
        Url:  "http://" + srv.Listener.Addr().String() + "/path",
        Body: data.Encode(),
        Headers: map[string][]string{
            "Content-Type": {"application/x-www-form-urlencoded"},
        },
    },
}

That should create a job that will do a POST with a form-encoded body. It can be done with the current version of Kala. Does that fit your use case?

tooolbox commented 4 years ago

@weburnit I saw that you closed this PR and made a new one for the broken dashboard, which is great.

I hope you can be successful at sending a form-encoded notification with Kala. After fixing the dashboard, please let me know if you still have a problem with form-encoding.