[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-718919467 @rmetzger I believe we should be good to merge. Thanks a lot @XComp, @vthinkxie for your time and patience. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-718706391 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-717796453 > Thanks @gm7y8 . Additionally, I verified manually that the changes result in the expected behavior. Let's wait for @vthinkxie to get back to us to review the code itself. @XComp @vthinkxie I just verified the fix after making suggested enhancements. it is working as expected. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-717783238 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-715584720 @XComp @vthinkxie I have addressed the requested changes. Build also has passed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-715456178 @XComp For some reason its failing in the HBase repo i retriggered the build This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-715452694 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-714289878 @XComp You are welcome! I have also verified fix with Word Count job ![Uploading Screen Shot 2020-10-22 at 12.23.01 AM.png…]() This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-714249342 > > > @XComp I was able to identify the code fix. I working to unit test it had some issue with Flink set up to start a job with checkpoint and savepoint. it might take a day or so as I am doing it for the first time. > > > > > > Hi @gm7y8 , thanks for getting back to us. Let's see if we can finish this before the codefreeze next week. Here a few remarks: > > > > * The build is failing for me locally (`mvn -pl flink-runtime-web -Dfast -DskipTests install`) with the following error message: > > > > ``` > > [ERROR] ERROR in src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkPointConfig' does not exist on type 'JobCheckpointsDetailComponent'. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > > ``` > > > > > > It's always good run a final `mvn install` at least on the modules you touched to verify your changes. Alternatively (or rather as an additional tool), you can setup Azure CI on your fork. That will run a full test covering all modules for each commit on your fork. Check out the [tutorial for setting up Azure CI](https://cwiki.apache.org/confluence/display/FLINK/Azure+Pipelines#AzurePipelines-Tutorial:SettingupAzurePipelinesforaforkoftheFlinkrepository). > > > > * The changes like [job-checkpoints-detail.component.html](https://github.com/apache/flink/pull/13458/commits/0b8e369aa5c8c4e4a8c370b4b848c0f197fcdb9d#diff-a1c50fd814fe8a1c03567f9c71877873251223cfaf32c7067068f7f30d8c5c42R25) does not reflect @AHeise's request as you use uppercased tokens here. @AHeise suggested normal casing (i.e. lowercase) as it helps to improve the readability. > > * Additionally, I realized that you didn't address all points of the PR description template. May you fix that? > > > > Thanks. > > Sure, I fixed the build issue u mentioned, and the build has passed now. Also will change the letter to lowercase. Also updated the description to reflect PR guidelines.. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-714126117 @flinkbot approve description This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-714118740 > > @XComp I was able to identify the code fix. I working to unit test it had some issue with Flink set up to start a job with checkpoint and savepoint. it might take a day or so as I am doing it for the first time. > > Hi @gm7y8 , thanks for getting back to us. Let's see if we can finish this before the codefreeze next week. Here a few remarks: > > * The build is failing for me locally (`mvn -pl flink-runtime-web -Dfast -DskipTests install`) with the following error message: > > ``` > [ERROR] ERROR in src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkPointConfig' does not exist on type 'JobCheckpointsDetailComponent'. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Element implicitly has an 'any' type because type 'CheckPointDetailInterface' has no index signature. > [ERROR] src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html(25,38): : Property 'checkpoint_type' does not exist on type 'JobCheckpointsDetailComponent'. > ``` > > It's always good run a final `mvn install` at least on the modules you touched to verify your changes. Alternatively (or rather as an additional tool), you can setup Azure CI on your fork. That will run a full test covering all modules for each commit on your fork. Check out the [tutorial for setting up Azure CI](https://cwiki.apache.org/confluence/display/FLINK/Azure+Pipelines#AzurePipelines-Tutorial:SettingupAzurePipelinesforaforkoftheFlinkrepository). > > * The changes like [job-checkpoints-detail.component.html](https://github.com/apache/flink/pull/13458/commits/0b8e369aa5c8c4e4a8c370b4b848c0f197fcdb9d#diff-a1c50fd814fe8a1c03567f9c71877873251223cfaf32c7067068f7f30d8c5c42R25) does not reflect @AHeise's request as you use uppercased tokens here. @AHeise suggested normal casing (i.e. lowercase) as it helps to improve the readability. > * Additionally, I realized that you didn't address all points of the PR description template. May you fix that? > > Thanks. Sure, I fix the build issue u mentioned, and the build has passed now. Also will change the letter to lowercase. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-712711411 @XComp I was able to identify the code fix. I working to unit test it had some issue with Flink set up to start a job with checkpoint and savepoint. it might take a day or so as I am doing it for the first time. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-708157763 @AHeise sorry for the delay to respond back. it should simple fix in the UI layer. I would take the opinion of @XComp @vthinkxie if it ok with the above change. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-704014916 > Thanks for this improvement @gm7y8 > Overall, it looks good to me. > > @XComp @vthinkxie Did someone check that the change works in UI? > > I left comments about changes which are probably unrelated. > After addressing these comments/question, I can merge the PR. > > Also, not sure what the first 'merge' commit means, I can remove it before merging. First Merge commit is created when I synced fork with the main repo. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-703980529 @XComp @azagrebin created to fix bug FLINK-19501 to fix the missing enum.. other PR just addresses only the html part not the snapshot document This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-700174911 @XComp I have taken care of commit message and other formatting issues. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-698158028 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-698158028 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] gm7y8 commented on pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI
gm7y8 commented on pull request #13458: URL: https://github.com/apache/flink/pull/13458#issuecomment-697169263 @flinkbot run azure This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
