Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1338#issuecomment-158765098
@rmetzger
1. I have introduced a separate directory for uploads, named randomly, and
deleted as part of the shut down hook.
2. I also tried closing the `URLClassLoader`, but it still doesn't allow
for the Jars to be deleted. I will look further into it and see if I can get it
to work. It shouldn't be a blocker however for merging this IMO.
@StephanEwen
1. I have removed the key to specify the upload directory, which takes care
of naming of the config entry. It now is just `jobmanager.web.submit.allow`
2. What would be the best way to separate the passing of path and query
parameters? Of course, the most obvious choice is to change the method
signature for `handleRequest` but it appears too much of a change, just to
allow query params. Since the interface is designed by us, I think we can keep
them together, and disallow any query param which overrides the path param.
Otherwise, if we do indeed change this, I suggest a construct `Parameters` with
fields `pathParams` and `queryParams`, with access method as `getParam(key,
enum{PATH, QUERY})` This will be a much cleaner solution.
3. I have some concerns about error handling. There are four handlers:
a. `StaticFileServerHandler`: Handles exception events itself, by sending
an `INTERNAL_SERVER_ERROR`
b. `RuntimeMonitorHandler`: Handles all exceptions itself, with either a
`NOT_FOUND` or `INTERNAL_SERVER_ERROR` code.
c. `HttpRequestHandler` [introduced in this PR]: Doesn't handler
exceptions. But I'm inclined towards sending an `INTERNAL_SERVER_ERROR` code
for any exceptions here.
d. `PipelineErrorHandler` [introduced in this PR]: If an exception caught
event is fired here, it can only happen because the netty threw an error [as
reported by @gyfora in an instance]. There's really nothing to do here except
sending an internal server error.
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
