metafacture-core
metafacture-core copied to clipboard
Add HttpPoster (#460)
Supersedes https://github.com/metafacture/metafacture-core/pull/461.
Why you don't like it?
I like the clearness about it (extending an an 'AbstractUrlConnection
', not just any HttpOpener
).
Granted, one more class is introduced , but then I like very much small classes.
hesitant about transporting both success and error data over the same channel
Ok. What's your proposal? Just passing an empty response and an throwing an Exception?
Granted, one more class is introduced , but then I like very much small classes.
But then you'd have to introduce new classes (and Flux commands) for every other verb we'd like to support (put-http
, delete-http
, ...). I like flexibility, I guess ;)
Although I realise that HttpOpener
and HttpPoster
accept different types of input: the URL and the request body, resp.
Maybe we could introduce a placeholder for the input data, e.g. @-
as in curl:
"<some url>"
|open-http() # default: method="get", url="@-"
"<some data>"
|open-http(url="<some url>", method="post") # default: body="@-"
Ok. What's your proposal? Just passing an empty response and an throwing an Exception?
I don't really have one right now. Maybe something akin to what we're doing in Fix functions: Let the user control the error case, e.g. with a prefix (that can have a default in this case).
But then you'd have to introduce new classes (and Flux commands) for every other verb we'd like to support (put-http, delete-http, ...).
But I like that, because these commands are soo self explaining (and documentation (i.e. usability) is one of our weaknesses). Many parameters make things so complicated (in my view).
Hm,hm. Maybe you have point. So this boils down to have only one class: HttpOpener
, configurable as GET/POST/PUT/DELETE ...
But then, what if we have other classes making use of URLConnection, like https://github.com/linked-swissbib/swissbib-metafacture-commands/blob/master/src/main/java/org/swissbib/linked/mf/source/MultiHttpOpener.java (I guess we could do this reusing HttpOpener
) or some SitemapReader (just heard about, don't know if this actually exists (@TobiasNx do you know?)).
[edit: when using other protocols then http(s)
an AbstractURLConnection
would be handy - but I guess we never will have a usage for other protocols?)
Many parameters make things so complicated (in my view).
That's hard to argue with. With great flexibility comes great responsibility ;) But then again, we'd have a unified interface instead of different classes with different behaviour and options.
But then, what if we have other classes making use of URLConnection
I'm not sure I follow. If you can subclass AbstractUrlConnection
, you can also subclass HttpOpener
? [Well, if it weren't for those pesky final
classes :( But that's a personal pet peeve of mine. I hate that you can't subclass any Metafacture classes.]
Not to rain on your parade, but this is the direction I had in mind: 3f7e150.
Maybe this helps to further the discussion.
Opened your branch #463. I like it , just added a content type header. Shall we close this branch and further discuss at #463?
Shall we close this branch and further discuss at #463?
If you think that's the way to go, then yes. But it wasn't my intention to kill the discussion here; you had a valid point with "multiple simple commands" vs. "one complex command". I just wanted to illustrate my point of view.
some SitemapReader (just heard about, don't know if this actually exists (@TobiasNx do you know?))
Yes, it's in oersi-etl (impl, test) and we have a card in our sprint backlog to move it to metafacture-core.
Superseded by #463.