You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by HeartSaVioR <gi...@git.apache.org> on 2015/12/22 07:23:51 UTC
[GitHub] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/563
ZEPPELIN-528 REST API: add "progress" to 'get notebook job' API
### What is this PR for?
To add 'progress' to 'get notebook job' REST API.
All paragraphs which status is 'running' will include 'progress' to their job status.
### What type of PR is it?
Improvement
### Todos
### Is there a relevant Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-528
### How should this be tested?
1. Run a notebook or paragraph which takes some times. Paragraph which could show progress (%) would be more appreciated. Confirm that its status is 'RUNNING'.
2. Call ```http://<zeppelin host>:<zeppelin port>/api/notebook/job/<notebook id>``` to see if it has 'progress' field in JSON.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? (No)
* Is there breaking changes for older versions? (No)
* Does this needs documentation? (Yes or no. Output of the JSON format could be updated, but it could be treated as not mandatory.)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HeartSaVioR/incubator-zeppelin ZEPPELIN-528
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/563.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 #563
----
commit 2597ebfcdcb48fa0ca6175bf2f461782fd267181
Author: Jungtaek Lim <ka...@gmail.com>
Date: 2015-12-22T05:34:58Z
ZEPPELIN-528 Add 'progress' to paragraph information when running
* It would be hard to trap RUNNING state, so I can't add UT
----
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-167065178
Great. Looks good to me.
Merge if there're no more discussions
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-166755709
@Leemoonsoo Addressed your comment.
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-166718539
Thanks for the good improvement.
I think it's better add an example of the JSON output that includes 'progress' field.
Could you give me a hint that what 'UT' is?
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/563
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-167047627
@Leemoonsoo Thanks for merging ZEPPELIN-527! Upmerged and add UT.
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-166752028
UT = Unit test :)
I'll address the doc and let you know when complete.
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-166854142
Thanks! LGTM
If you would like to add UT in ZeppelinRestApiTest, you can add a such paragraph ```%sh sleep 1``` instead of https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java#L401 and state transition READY->PENDING->RUNNING->FINISHED will stay in RUNNING state for a second. Does it make sense ?
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-166528935
Two questions here,
- Can I trap RUNNING status of the paragraph in UT? I don't have an idea to make it, so I can't add UT for now.
- Would it be better to modify output of the JSON format to include 'progress' field?
---
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] incubator-zeppelin pull request: ZEPPELIN-528 REST API: add "progr...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/563#issuecomment-167026319
@Leemoonsoo
Thanks for sharing the idea! It works like a charm!
But new unit test requires ZEPPELIN-527 to be merged since API returns HTTP STATUS 500.
There's a bit conflict between ZEPPELIN-527 and ZEPPELIN-528 so I'll wait ZEPPELIN-527 to be merged and add commit regarding UT to ZEPPELIN-528.
Could you please take a look at ZEPPELIN-527 first? Thanks!
---
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.
---