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

[FLINK-7806] [flip6] Register CurrentJobsOverviewHandler under /jobs #4805

Closed

Conversation

tillrohrmann
Copy link
Contributor

What is the purpose of the change

This PR changes the REST endpoint URL of the CurrentJobsOverviewHandler from /joboverview to /jobs/overview. Moreover, it renames the handler to JobsOverviewHandler.

Brief change log

  • Rename CurrentJobsOverviewHandler to JobsOverviewHandler
  • Change REST handler paths
  • Remove joboverview/running and joboverview/completed from JobsOverviewHandler
  • Adapt web ui files

Verifying this change

  • The change has been manually tested

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@tillrohrmann tillrohrmann force-pushed the moveCurrentJobsOverviewHandler branch 2 times, most recently from 2a9c951 to b5f670d Compare October 11, 2017 16:26
Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

the vendor.js and vendor.css files should not change; otherwise +1-

@tillrohrmann
Copy link
Contributor Author

Not sure why the changed @zentol. I just executed the build process as it was described in the readme.

@tillrohrmann tillrohrmann force-pushed the moveCurrentJobsOverviewHandler branch 2 times, most recently from 59a21af to 2d41896 Compare October 11, 2017 22:50
@zentol
Copy link
Contributor

zentol commented Oct 12, 2017

Often happens due to different dependency/build system versions of even OS specific oddities. I typically just revert those changes.

@tillrohrmann
Copy link
Contributor Author

Alright. I'm wondering whether it would be possible to somehow fix these things because this is bound to be changed.

@tillrohrmann
Copy link
Contributor Author

I reverted the changes to vendor.css and vendor.js and updated the rest_api.md.

@tillrohrmann
Copy link
Contributor Author

I'm not entirely sure, but I think we cannot put the JobsOverviewHandler under /jobs/overview. The problem is that it collides with /jobs/:jobid. If I access http://localhost:8081/jobs/overview I get a java.lang.IllegalArgumentException: contains illegal character for hexBinary: overview. Not sure why it works when the call comes from the web ui.

So either we don't do this change or we register the handler under /jobs. Then we would simply return a detailed view when accessing jobs. In the future we could add an optional filter statement to not include all information. What do you think @zentol?

@zentol
Copy link
Contributor

zentol commented Oct 13, 2017

let's put iut under /jobs then for now.

@tillrohrmann
Copy link
Contributor Author

Alright, I'll change the PR.

@tillrohrmann tillrohrmann force-pushed the moveCurrentJobsOverviewHandler branch 2 times, most recently from 05fa39c to 252a0d8 Compare October 16, 2017 08:54
@tillrohrmann tillrohrmann changed the title [FLINK-7806] [flip6] Register CurrentJobsOverviewHandler under /jobs/overview [FLINK-7806] [flip6] Register CurrentJobsOverviewHandler under /jobs Oct 20, 2017
@tillrohrmann
Copy link
Contributor Author

Rebased onto the latest master. Once Travis gives green light, I'll merge this PR.

The AkkaOptions.RETRY_GATE_CLOSED_FOR allows to configure how long a remote
ActorSystem is gated in case of a connection loss. The default value is set
to 50 ms.

This closes apache#4903.
HandlerUtils#sendResponse now accepts a map of additional http response headers
and their values. This allows to set additional headers such as the
ACCESS_CONTROL_ALLOW_ORIGIN header and its value.

This closes apache#4859.
Send dataPort and HardwareDescription to RM

Instantiate RM leader retriever
…sages to rest.messages.taskmanager

Move TaskManager messages to rest.messages.taskmanager

Move TaskManager message tests to rest.messages.taskmanager
Pass MetricQueryServiceRetriever to DispatcherRestEndpoint

This closes apache#4862.
tillrohrmann and others added 2 commits November 3, 2017 15:58
Add JobID(De)Serializer and JobVertexID(De)Serializer for jackson

This closes apache#4884.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Nov 3, 2017
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes apache#4805.
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes apache#4805.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Nov 6, 2017
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes apache#4805.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Nov 7, 2017
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes apache#4805.
asfgit pushed a commit that referenced this pull request Nov 7, 2017
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes #4805.
@tillrohrmann tillrohrmann deleted the moveCurrentJobsOverviewHandler branch November 7, 2017 14:39
GJL pushed a commit to GJL/flink that referenced this pull request Nov 8, 2017
…overview

Rename CurrentJobsOverviewHandler to JobsOverviewHandler

Change paths

Remove joboverview/running and joboverview/completed from JobsOverviewHandler

Adapt web ui files

Update rest_api to reflect new REST call /jobs

This changes apache#4805.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants