You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by uce <gi...@git.apache.org> on 2015/12/09 16:51:10 UTC
[GitHub] flink pull request: [FLINK-2796] [runtime-web] Add configurable jo...
GitHub user uce opened a pull request:
https://github.com/apache/flink/pull/1449
[FLINK-2796] [runtime-web] Add configurable job manager address to HTTP requests
These changes (re-)enable testing of changes to the web frontend:
- Set the `allow-origin` header in order to allow requests from the test server (`localhost:3000` requests to `localhost:8081`). I don't know how this was working before? We never set that before.
- Add job manager address to get requests and set default to empty String. This was completely removed in 3b8b4f0f8c0600dc851d676ce1bd7f5ab81cb64f, but it should have been only set to the empty string.
All `$http.get` requests should refer to the `jobServer` variable.
```
grep -r "\$http.get" flink-runtime-web/web-dashboard/app/
flink-runtime-web/web-dashboard/app//scripts/common/services.coffee: $http.get flinkConfig.jobServer + "config"
flink-runtime-web/web-dashboard/app//scripts/modules/jobmanager/jobmanager.svc.coffee: $http.get(flinkConfig.jobServer + "jobmanager/config")
flink-runtime-web/web-dashboard/app//scripts/modules/jobmanager/jobmanager.svc.coffee: $http.get(flinkConfig.jobServer + "jobmanager/log")
flink-runtime-web/web-dashboard/app//scripts/modules/jobmanager/jobmanager.svc.coffee: $http.get(flinkConfig.jobServer + "jobmanager/stdout")
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "joboverview"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + jobid
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + jobid + "/config"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + currentJob.jid + "/vertices/" + vertexid + "/subtasktimes"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + currentJob.jid + "/vertices/" + vertexid
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + currentJob.jid + "/vertices/" + vertexid + "/accumulators"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + currentJob.jid + "/vertices/" + vertexid + "/subtasks/accumulators"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + currentJob.jid + "/exceptions"
flink-runtime-web/web-dashboard/app//scripts/modules/jobs/jobs.svc.coffee: $http.get flinkConfig.jobServer + "jobs/" + jobid + "/yarn-cancel"
flink-runtime-web/web-dashboard/app//scripts/modules/overview/overview.svc.coffee: $http.get(flinkConfig.jobServer + "/overview")
flink-runtime-web/web-dashboard/app//scripts/modules/taskmanager/taskmanager.svc.coffee: $http.get(flinkConfig.jobServer + "taskmanagers")
flink-runtime-web/web-dashboard/app//scripts/modules/taskmanager/taskmanager.svc.coffee: $http.get(flinkConfig.jobServer + "taskmanagers/" + taskmanagerid)
```
---
**If you can find the time, make sure to test this by really running the frontend and not only by looking at the diffs. We don't have test coverage for these parts...**
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/uce/flink 2796-web_test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1449.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1449
----
commit 045da2cd619d51ede7bb7646fff4ba3620f619dd
Author: Ufuk Celebi <uc...@apache.org>
Date: 2015-12-09T15:36:35Z
[FLINK-2796] [runtime-web] Set allow-origin header
Many browsers don't allow cross-origin HTTP requests if the respective
HTTP header is not set by the server.
Because of this it was not possible to test changes to the web frontend
with the local proxy server and a running job manager.
See here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS
commit 9ad26e9aeb6e2eb26f5155ad3c290e00d3421cc9
Author: Ufuk Celebi <uc...@apache.org>
Date: 2015-12-09T15:41:07Z
[FLINK-2796] [runtime-web] Add configurable job manager address to HTTP requests
This was removed in 3b8b4f0f8c0600dc851d676ce1bd7f5ab81cb64f. The
issue was fixed by this, but it disabled local testing as well.
This change re-introduces the variable and sets it to the empty
string by default. This way, we can still use the proxy server for
local testing.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1449
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/1449#discussion_r47291227
--- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/overview/overview.svc.coffee ---
@@ -24,7 +24,7 @@ angular.module('flinkApp')
@loadOverview = ->
deferred = $q.defer()
- $http.get("overview")
+ $http.get(flinkConfig.jobServer + "/overview")
--- End diff --
Good catch. Was not on purpose
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1449#issuecomment-163953942
I think its good to merge, +1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/1449#issuecomment-163893598
Thanks for the reviews Robert and Sachin!
I've addressed Robert's comment. If there are no objections, I would like to merge this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1449#discussion_r47281222
--- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/overview/overview.svc.coffee ---
@@ -24,7 +24,7 @@ angular.module('flinkApp')
@loadOverview = ->
deferred = $q.defer()
- $http.get("overview")
+ $http.get(flinkConfig.jobServer + "/overview")
--- End diff --
absolute url's (starting with '/') will break the YARN proxy stuff.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2796] [runtime-web] Add configurable jo...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1449#issuecomment-163570155
Wrong JIRA issue?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by uce <gi...@git.apache.org>.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/1449#issuecomment-163594437
Thanks! It was 2769 and not 2796. Updated the PR.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2769] [runtime-web] Add configurable jo...
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1449#issuecomment-163866658
Tested it out locally. Works well with `gulp watch` and custom web frontend port.
A few things I noticed:
1. Job Manager's log and stdout are not accessible with CORS, since they're served via `StaticFileServerHandler` which doesn't set CORS headers. This however shouldn't be an issue. We need to have a better logs service anyway, for both Job manager and task managers IMO.
2. At some point, we will remove the `GET /jobs/<job-id>/yarn-cancel` method, which is only there due to a limitation of yarn. To this end, there needs to be an `OPTIONS` handlers for the `DELETE` request to send CORS response headers in the preflight phase. https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Preflighted_requests. Not for the foreseeable future though. :-')
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---