You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/12/09 15:04:35 UTC

[GitHub] [cloudstack] DaanHoogland opened a new issue, #6966: remove instanceof checks and use reflection for admin cmd invocations

DaanHoogland opened a new issue, #6966:
URL: https://github.com/apache/cloudstack/issues/6966

   <!--
   Verify first that your issue/request is not already reported on GitHub.
   Also test if the latest release and main branch are affected too.
   Always add information AFTER of these HTML comments, but no need to delete the comments.
   -->
   
   ##### ISSUE TYPE
    * cleanup
   
   ##### COMPONENT NAME
   <!--
   Categorize the issue, e.g. API, VR, VPN, UI, etc.
   -->
   ~~~
   API
   ~~~
   
   ##### CLOUDSTACK VERSION
   <!--
   New line separated list of affected versions, commit ID for issues on main branch.
   -->
   
   ~~~
   4.17
   ~~~
   
   ##### CONFIGURATION
   <!--
   Information about the configuration if relevant, e.g. basic network, advanced networking, etc.  N/A otherwise
   -->
   
   
   ##### OS / ENVIRONMENT
   <!--
   Information about the environment if relevant, N/A otherwise
   -->
   
   
   ##### SUMMARY
   <!-- Explain the problem/feature briefly -->
   the API classes have instances that are for root admin only, these are checked for using `instanceof` in the service layer. This should be cleaned up in favour for reflection invocations as in #6951. It would be better to do all in one go but the fix in #6951 really needs it and is for a point release, so postponing.
   
   ##### STEPS TO REPRODUCE
   <!--
   For bugs, show exactly how to reproduce the problem, using a minimal test-case. Use Screenshots if accurate.
   
   For new features, show how the feature would be used.
   -->
   
   <!-- Paste example playbooks or commands between quotes below -->
   ~~~
   
   ~~~
   
   <!-- You can also paste gist.github.com links for larger files -->
   
   ##### EXPECTED RESULTS
   <!-- What did you expect to happen when running the steps above? -->
   
   ~~~
   
   ~~~
   
   ##### ACTUAL RESULTS
   <!-- What actually happened? -->
   
   <!-- Paste verbatim command output between quotes below -->
   ~~~
   
   ~~~
   


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] remove instanceof checks and use reflection for admin cmd invocations [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on issue #6966:
URL: https://github.com/apache/cloudstack/issues/6966#issuecomment-1742619443

   @DaanHoogland do you plan to work on this for 4.19 or can this moved to the next milestone?


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] remove instanceof checks and use reflection for admin cmd invocations [cloudstack]

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on issue #6966:
URL: https://github.com/apache/cloudstack/issues/6966#issuecomment-2298324579

   @DaanHoogland 
   is the issue related to #6951  ? (according to the summary)


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] remove instanceof checks and use reflection for admin cmd invocations [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #6966:
URL: https://github.com/apache/cloudstack/issues/6966#issuecomment-4598148449

   This issue is stale because it has been open for 120 days with no activity. It may be removed by administrators of this project at any time. Remove the stale label or comment to request for removal of it to prevent this.


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] remove instanceof checks and use reflection for admin cmd invocations [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on issue #6966:
URL: https://github.com/apache/cloudstack/issues/6966#issuecomment-2298355605

   > @DaanHoogland is the issue related to #6951 ? (according to the summary)
   
   by the looks of it this is a mis-reference . This is about using `if (cmd isinstance of ..CmdForAdmin)` conditions in the service layer. extremely low priority, but a cleanup we may have to consider sometime.


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on issue #6966: remove instanceof checks and use reflection for admin cmd invocations

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on issue #6966:
URL: https://github.com/apache/cloudstack/issues/6966#issuecomment-1365277009

   This may not be necessary generally; the metrics API extend relevant list APIs (for example, listVirtualMachines and listVirtualMachinesMetrics) and because of this the reflections-based pattern was used.
   
   Ideally one could have implemented that by implementing a interface that both the listVirtualMachines and listVirtualMachinesMetrics implements, and checking using `instanceof` on that interface; this would have introduced a static type checking and avoided the dynamic/runtime behaviour that reflection based code (generally) introduces.


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org