You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rashmidixit <gi...@git.apache.org> on 2017/01/05 06:58:08 UTC

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

GitHub user rashmidixit opened a pull request:

    https://github.com/apache/cloudstack/pull/1889

    CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone

    Refer to [CLOUDSTACK-9718](https://issues.apache.org/jira/browse/CLOUDSTACK-9718) for more details.
    
    Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
    confuse the user as the target host might never be shown in the UI.
    
    Root Cause: The API (findHostsForMigration) always returned page 1 results which would
    be always <= default.page.size global parameter. Therefore, in case of large
    no. of hosts where the result can map to multiple pages, this issue would arise.
    
    Solution: 1. Replace drop-down with listView widget.
    2. Allow lazy-loading of records on listView's scroll.
    3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
    4. Provide 'Search by host name' to limit the results.
    
    Conflicts:
    	client/WEB-INF/classes/resources/messages.properties
    	client/WEB-INF/classes/resources/messages_ja_JP.properties
    	client/WEB-INF/classes/resources/messages_zh_CN.properties
    	ui/css/cloudstack3.css
    	ui/dictionary2.jsp

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9718

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1889.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 #1889
    
----
commit dfb595bdfebc38ae1561584030e62c40cdbd46a5
Author: radhika grover <ra...@accelerite.com>
Date:   2016-05-06T12:57:41Z

    CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone
    
    Reviewed-By: Rashmi Dixit
    
    Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
    confuse the user as the target host might never be shown in the UI.
    
    Root Cause: The API (findHostsForMigration) always returned page 1 results which would
    be always <= default.page.size global parameter. Therefore, in case of large
    no. of hosts where the result can map to multiple pages, this issue would arise.
    
    Solution: 1. Replace drop-down with listView widget.
    2. Allow lazy-loading of records on listView's scroll.
    3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
    4. Provide 'Search by host name' to limit the results.
    
    Conflicts:
    	client/WEB-INF/classes/resources/messages.properties
    	client/WEB-INF/classes/resources/messages_ja_JP.properties
    	client/WEB-INF/classes/resources/messages_zh_CN.properties
    	ui/css/cloudstack3.css
    	ui/dictionary2.jsp

----


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @karuturi I have rebased with master and pushed. I still see failures in Travis but they dont seem to be with my changes. Can you please check?


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1889


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r97313518
  
    --- Diff: ui/scripts/ui-custom/migrate.js ---
    @@ -0,0 +1,127 @@
    +// Licensed to the Apache Software Foundation (ASF) under one
    --- End diff --
    
    @koushik-das I have provided screen shots in JIRA. Attaching them here as well.
    ![migrateinstance-seehosts-search](https://cloud.githubusercontent.com/assets/13415006/22204992/fdfacbae-e19a-11e6-9c7b-59a0776c68e0.PNG)
    ![migrateinstance-seehosts](https://cloud.githubusercontent.com/assets/13415006/22204993/fdfaf232-e19a-11e6-8ea9-1af368f1f6d7.PNG)
    



---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
GitHub user rashmidixit reopened a pull request:

    https://github.com/apache/cloudstack/pull/1889

    CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone

    Refer to [CLOUDSTACK-9718](https://issues.apache.org/jira/browse/CLOUDSTACK-9718) for more details.
    
    Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
    confuse the user as the target host might never be shown in the UI.
    
    Root Cause: The API (findHostsForMigration) always returned page 1 results which would
    be always <= default.page.size global parameter. Therefore, in case of large
    no. of hosts where the result can map to multiple pages, this issue would arise.
    
    Solution: 1. Replace drop-down with listView widget.
    2. Allow lazy-loading of records on listView's scroll.
    3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
    4. Provide 'Search by host name' to limit the results.
    
    Conflicts:
    	client/WEB-INF/classes/resources/messages.properties
    	client/WEB-INF/classes/resources/messages_ja_JP.properties
    	client/WEB-INF/classes/resources/messages_zh_CN.properties
    	ui/css/cloudstack3.css
    	ui/dictionary2.jsp

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9718

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1889.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 #1889
    
----
commit d8158fe5f4bcdbe58fac4fde1969253f31cdce47
Author: rashmidixit <ra...@persistent.co.in>
Date:   2017-03-16T10:35:22Z

     CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone
    
        Reviewed-By: Rashmi Dixit
    
        Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
        confuse the user as the target host might never be shown in the UI.
    
        Root Cause: The API (findHostsForMigration) always returned page 1 results which would
        be always <= default.page.size global parameter. Therefore, in case of large
        no. of hosts where the result can map to multiple pages, this issue would arise.
    
        Solution: 1. Replace drop-down with listView widget.
        2. Allow lazy-loading of records on listView's scroll.
        3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
        4. Provide 'Search by host name' to limit the results.
    
    Added change where if there are no hosts found, an empty row with message will
    appear.

----


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    Nice work @rashmidixit, tested it and works great! LGTM


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    LGTM, did not test it.


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    I ported this to 4.7.1,and tested it. It works fine (list/order/search).
    LGTM


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1889


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @rashmidixit finally  I decide to say LGTM to this 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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r95973392
  
    --- Diff: ui/scripts/ui-custom/migrate.js ---
    @@ -0,0 +1,127 @@
    +// Licensed to the Apache Software Foundation (ASF) under one
    --- End diff --
    
    For UI changes please provide UI screenshots


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r101275471
  
    --- Diff: api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
    @@ -206,7 +206,7 @@ public void execute() {
             } else {
                 Pair<List<? extends Host>, Integer> result;
                 Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> hostsForMigration =
    -                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal());
    +                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal(), null);
    --- End diff --
    
    @koushik-das  One of the changes in this commit was to add keyword as part of the listHostsForMigrationOfVM API. Earlier in this place the keyword was not getting passed down. Since it wasnt passed earlier, to keep this call the same, passing null as input. Hope this explains. 


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    LGTM based on code review


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @ustcweizhou The popup is to indicate that no hosts were found. If I remove that, then there will be no visual indication that the search has returned no results. 
    
    We need to have an alternative way to show that 0 results found. Let me see if I can put a message somewhere beneath to say the same. Will get back to you in a day or so.
    
    Thanks for trying this out!


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @ustcweizhou thanks. I guess there was a misunderstanding.
    
    I understand what you are saying. I think i will just remove that dialog. The problem is that we are lazy loading the list of hosts. If there are many, as the user scrolls down, the page loads. 
    
    Can you tell me how many hosts you have in the list and explain a little about the problem you face? 


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @ustcweizhou I have made appropriate changes. Can you please check once the validations pass?


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @karuturi ping.


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
GitHub user rashmidixit reopened a pull request:

    https://github.com/apache/cloudstack/pull/1889

    CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone

    Refer to [CLOUDSTACK-9718](https://issues.apache.org/jira/browse/CLOUDSTACK-9718) for more details.
    
    Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
    confuse the user as the target host might never be shown in the UI.
    
    Root Cause: The API (findHostsForMigration) always returned page 1 results which would
    be always <= default.page.size global parameter. Therefore, in case of large
    no. of hosts where the result can map to multiple pages, this issue would arise.
    
    Solution: 1. Replace drop-down with listView widget.
    2. Allow lazy-loading of records on listView's scroll.
    3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
    4. Provide 'Search by host name' to limit the results.
    
    Conflicts:
    	client/WEB-INF/classes/resources/messages.properties
    	client/WEB-INF/classes/resources/messages_ja_JP.properties
    	client/WEB-INF/classes/resources/messages_zh_CN.properties
    	ui/css/cloudstack3.css
    	ui/dictionary2.jsp

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9718

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1889.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 #1889
    
----
commit e64306de19a011f7fef3d3a92d7d2bbc6792e215
Author: rashmidixit <ra...@persistent.co.in>
Date:   2017-03-16T10:35:22Z

     CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for migration in a Zone
    
        Reviewed-By: Rashmi Dixit
    
        Problem: All the hosts suitable for VM Migration are not shown in the UI. This could
        confuse the user as the target host might never be shown in the UI.
    
        Root Cause: The API (findHostsForMigration) always returned page 1 results which would
        be always <= default.page.size global parameter. Therefore, in case of large
        no. of hosts where the result can map to multiple pages, this issue would arise.
    
        Solution: 1. Replace drop-down with listView widget.
        2. Allow lazy-loading of records on listView's scroll.
        3. Show additional parameters (CPU/Memory used) to assist admin in decision making.
        4. Provide 'Search by host name' to limit the results.
    
    Added change where if there are no hosts found, an empty row with message will
    appear.

----


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @rashmidixit thanks for the update.
    Compared with old patch, I notice you changed the return if args.page == 1.
    However, it does not solve my issue. We have several hosts (more than 1 page). Each time when I get the last page (full list),  I get the alert dialog, that is what I want to avoid.
    



---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @rashmidixit  travis is failing. Can you rebase with master and force push?


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r101313545
  
    --- Diff: ui/scripts/ui-custom/migrate.js ---
    @@ -0,0 +1,127 @@
    +// Licensed to the Apache Software Foundation (ASF) under one
    --- End diff --
    
    the screenshot looks awesome!!!


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @rashmidixit thanks for your work.


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    Looks good (did not test it)


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r101275818
  
    --- Diff: api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
    @@ -206,7 +206,7 @@ public void execute() {
             } else {
                 Pair<List<? extends Host>, Integer> result;
                 Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> hostsForMigration =
    -                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal());
    +                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal(), null);
    --- End diff --
    
    @koushik-das One of the changes in this commit was to add keyword as part of the listHostsForMigrationOfVM API. Earlier in this place the keyword was not getting passed down. Since it wasnt passed earlier, to keep this call the same, passing null as input. Hope this explains.


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by rashmidixit <gi...@git.apache.org>.
Github user rashmidixit commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @karuturi requesting you to merge these changes.


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    LGTM, please merge it.


---
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] cloudstack issue #1889: CLOUDSTACK-9718: Revamp the dropdown showing lists o...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1889
  
    @rashmidixit We have ported it in our branch and found it inconvenient to click the OK button in dialog with warning "No more hosts are available for migration" frequently.
    
    could you remove the line below ?
    ```
                                                             cloudStack.dialog.notice({
                                                                 message: _l('message.no.host.available')
                                                             }); //Only a single host in the set up
    -                                                    } else {
    -                                                         cloudStack.dialog.notice({
    -                                                             message: _l('message.no.more.hosts.available')
    -                                                         });
                                                         }
                                                     }
                                                 });
    ```


---
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] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1889#discussion_r95973044
  
    --- Diff: api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
    @@ -206,7 +206,7 @@ public void execute() {
             } else {
                 Pair<List<? extends Host>, Integer> result;
                 Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Host, Boolean>> hostsForMigration =
    -                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal());
    +                _mgr.listHostsForMigrationOfVM(getVirtualMachineId(), this.getStartIndex(), this.getPageSizeVal(), null);
    --- End diff --
    
    Why keyword is not used here? All API commands derived from BaseListCmd should have keyword.


---
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.
---