Details

    • Sub-task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.7.1, 0.8.1-alpha
    • UI
    • None

    Description

      Modify table component to automatically update cell based on in-progress data.
      #. Upgrade polling logic to manage a specific entity.
      #. Added progress column to All Tasks table.
      #. Updated table to automatically reflect model change - Just need to set observePath to true in defaultColumnConfigs.
      #. Updated table logic to send action on row change - so that polling logic can query for fresh records.

      Attachments

        1. TEZ-2780.1.patch
          28 kB
          Sreenath Somarajapuram
        2. TEZ-2780.2.patch
          17 kB
          Sreenath Somarajapuram
        3. TEZ-2780.3.patch
          17 kB
          Sreenath Somarajapuram
        4. TEZ-2780.4.patch
          17 kB
          Sreenath Somarajapuram
        5. TEZ-2780.wip.1.patch
          7 kB
          Sreenath Somarajapuram

        Issue Links

          Activity

            Added REST API for getting task details.
            Query params: dagID, [vertexID, taskIndex, limit]
            [] - Optional
            Details returned - id, progress & status

            Sreenath Sreenath Somarajapuram added a comment - Added REST API for getting task details. Query params: dagID, [vertexID, taskIndex, limit] [] - Optional Details returned - id, progress & status
            Sreenath Sreenath Somarajapuram added a comment - - edited

            1. Added AM API for fetching realtime tasks info.
            2. Upgraded pollster to manage polling for a specific entity.
            3. Added progress column to All Tasks table.
            4. Updated table to automatically reflect model change

            • Just need to set observePath to true in defaultColumnConfigs.

            5. Updated table logic to send action on row change.

            pramachandran Please review.

            Sreenath Sreenath Somarajapuram added a comment - - edited 1. Added AM API for fetching realtime tasks info. 2. Upgraded pollster to manage polling for a specific entity. 3. Added progress column to All Tasks table. 4. Updated table to automatically reflect model change Just need to set observePath to true in defaultColumnConfigs. 5. Updated table logic to send action on row change. pramachandran Please review.

            Sreenath still reviewing the changes, one quick question though. Why does the polling class need to be a mixin? this has two disadvantages.
            1. there can be only a single instance of the polling.
            2. its tied to the type of controller it is used in.
            IMHO it would be better if its not a mixin, instantiate it in init. and depending on if polling is required, do a start/stop. if you are concerned about code reuse across task/task attempts write a helper function that takes in the required arguments.

            pramachandran Prakash Ramachandran added a comment - Sreenath still reviewing the changes, one quick question though. Why does the polling class need to be a mixin? this has two disadvantages. 1. there can be only a single instance of the polling. 2. its tied to the type of controller it is used in. IMHO it would be better if its not a mixin, instantiate it in init. and depending on if polling is required, do a start/stop. if you are concerned about code reuse across task/task attempts write a helper function that takes in the required arguments.
            tezqa TezQA added a comment -

            -1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12754671/TEZ-2780.1.patch
            against master revision c07d6b7.

            +1 @author. The patch does not contain any @author tags.

            -1 tests included. The patch doesn't appear to include any new or modified tests.
            Please justify why no new tests are needed for this patch.
            Also please list what manual steps were performed to verify this patch.

            +1 javac. The applied patch does not increase the total number of javac compiler warnings.

            +1 javadoc. There were no new javadoc warning messages.

            +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

            +1 release audit. The applied patch does not increase the total number of release audit warnings.

            +1 core tests. The patch passed unit tests in .

            Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1086//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1086//console

            This message is automatically generated.

            tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12754671/TEZ-2780.1.patch against master revision c07d6b7. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1086//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1086//console This message is automatically generated.
            hitesh Hitesh Shah added a comment -

            It would be good if the AM webservices and the UI changes could be split out into 2 jiras.

            hitesh Hitesh Shah added a comment - It would be good if the AM webservices and the UI changes could be split out into 2 jiras.

            hitesh Sure. Moving the API part to TEZ-2792. This ticket would be just for UI changes.

            Sreenath Sreenath Somarajapuram added a comment - hitesh Sure. Moving the API part to TEZ-2792 . This ticket would be just for UI changes.

            Canceling as REST API was moved to TEZ-2792 & to address comments.

            Sreenath Sreenath Somarajapuram added a comment - Canceling as REST API was moved to TEZ-2792 & to address comments.

            pramachandran

            • The mixin was created for all table pages with the assumption that only the respective entity type would have to be polled.
            • It was created with the intention to add polling functionality to a table controller with the least amount of code.
            • That said, I totally agree that on considering a future use-case of multiple polling this would be of less use. Hence I have moved all the functionalities into a new class named EntityArrayPollster (inherited from pollster).

            The changes can be found in patch 2. Please review.

            Sreenath Sreenath Somarajapuram added a comment - pramachandran The mixin was created for all table pages with the assumption that only the respective entity type would have to be polled. It was created with the intention to add polling functionality to a table controller with the least amount of code. That said, I totally agree that on considering a future use-case of multiple polling this would be of less use. Hence I have moved all the functionalities into a new class named EntityArrayPollster (inherited from pollster). The changes can be found in patch 2. Please review.
            tezqa TezQA added a comment -

            -1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12754966/TEZ-2780.2.patch
            against master revision 00508f8.

            +1 @author. The patch does not contain any @author tags.

            -1 tests included. The patch doesn't appear to include any new or modified tests.
            Please justify why no new tests are needed for this patch.
            Also please list what manual steps were performed to verify this patch.

            +1 javac. The applied patch does not increase the total number of javac compiler warnings.

            +1 javadoc. There were no new javadoc warning messages.

            +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

            +1 release audit. The applied patch does not increase the total number of release audit warnings.

            +1 core tests. The patch passed unit tests in .

            Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1099//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1099//console

            This message is automatically generated.

            tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12754966/TEZ-2780.2.patch against master revision 00508f8. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1099//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1099//console This message is automatically generated.

            Making the patch compatible with TEZ-2792.

            Sreenath Sreenath Somarajapuram added a comment - Making the patch compatible with TEZ-2792 .
            tezqa TezQA added a comment -

            -1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12755097/TEZ-2780.3.patch
            against master revision 00508f8.

            +1 @author. The patch does not contain any @author tags.

            -1 tests included. The patch doesn't appear to include any new or modified tests.
            Please justify why no new tests are needed for this patch.
            Also please list what manual steps were performed to verify this patch.

            +1 javac. The applied patch does not increase the total number of javac compiler warnings.

            +1 javadoc. There were no new javadoc warning messages.

            +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

            +1 release audit. The applied patch does not increase the total number of release audit warnings.

            +1 core tests. The patch passed unit tests in .

            Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1104//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1104//console

            This message is automatically generated.

            tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12755097/TEZ-2780.3.patch against master revision 00508f8. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1104//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1104//console This message is automatically generated.

            Canceling to address comments.

            Sreenath Sreenath Somarajapuram added a comment - Canceling to address comments.

            Attaching a fresh patch to:
            1. Solve the encoding issue observed in Prakash's setup
            2. Stop polling when no records are displayed on using a filter

            pramachandran Please review.

            Sreenath Sreenath Somarajapuram added a comment - Attaching a fresh patch to: 1. Solve the encoding issue observed in Prakash's setup 2. Stop polling when no records are displayed on using a filter pramachandran Please review.

            +1 LGTM pending pre-commit. tested with older version of yarn which had issues.

            pramachandran Prakash Ramachandran added a comment - +1 LGTM pending pre-commit. tested with older version of yarn which had issues.
            tezqa TezQA added a comment -

            -1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12755349/TEZ-2780.4.patch
            against master revision b288be7.

            +1 @author. The patch does not contain any @author tags.

            -1 tests included. The patch doesn't appear to include any new or modified tests.
            Please justify why no new tests are needed for this patch.
            Also please list what manual steps were performed to verify this patch.

            +1 javac. The applied patch does not increase the total number of javac compiler warnings.

            +1 javadoc. There were no new javadoc warning messages.

            +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

            +1 release audit. The applied patch does not increase the total number of release audit warnings.

            +1 core tests. The patch passed unit tests in .

            Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1113//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1113//console

            This message is automatically generated.

            tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12755349/TEZ-2780.4.patch against master revision b288be7. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1113//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1113//console This message is automatically generated.

            Thanks pramachandran.
            Committed to master and 0.7.

            Sreenath Sreenath Somarajapuram added a comment - Thanks pramachandran . Committed to master and 0.7.

            People

              Sreenath Sreenath Somarajapuram
              Sreenath Sreenath Somarajapuram
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: