You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "Christian Schneider (JIRA)" <ji...@apache.org> on 2012/07/11 10:49:34 UTC

[jira] [Created] (FELIX-3590) Add system property resolution to CommandSessionImpl

Christian Schneider created FELIX-3590:
------------------------------------------

             Summary: Add system property resolution to CommandSessionImpl
                 Key: FELIX-3590
                 URL: https://issues.apache.org/jira/browse/FELIX-3590
             Project: Felix
          Issue Type: Improvement
    Affects Versions: gogo.runtime-0.10.0
            Reporter: Christian Schneider
             Fix For: gogo.runtime-0.12.0


Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.

See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.

In the innermost code it is only the following we override in CommandSessionImpl.
        public Object get(String name) {
            Object val = super.get(name);
            if (val == null) {
                val = System.getProperty(name);
            }
            return val;
        }

So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.

If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13411954#comment-13411954 ] 

Christian Schneider commented on FELIX-3590:
--------------------------------------------

While in theory you are right I think in practice System properties are normally not changed at runtime. So session will most times not influence each other. Still I can imagine that not everyone wants the system property resolution.

On the other hand we currently need this feature in Karaf. So perhaps we can find some kind of extensibility mechanism that Karaf can use to add this to gogo. Basically we would like to tap into the get(String name) resolution. How about having an interface we can implement and inject into the session. 

Like: interface VariableResolver {
  Object resolve(String name);
}

and session.addVariableResolver(VariableResolver resolver)

The implementation of get in session would call the resolver if it is present. So we could change the behaviour for Karaf and it would still stay the same by default for Felix. Does that make sense?

                
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13411537#comment-13411537 ] 

Richard S. Hall commented on FELIX-3590:
----------------------------------------

It is not clear to me that this is a good idea in general. Aren't these supposed to be session properties? By definition, system properties would span sessions which means that one session could mess with another, which isn't always good when a session doesn't expect some property values to change.

For similar reasons, we avoid using system properties for framework configuration. The way we provide system properties to the framework is to have the launcher copy the system properties into the framework configuration properties (i.e., take a snapshot and give existing config properties priority over the system properties). That way different framework instances cannot interfere with each other.
                
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Christian Schneider resolved FELIX-3590.
----------------------------------------

    Resolution: Won't Fix

I have checked with Guillaume. It is good enough for us to copy the system properties on session start
                
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Closed] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Richard S. Hall closed FELIX-3590.
----------------------------------

    
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Richard S. Hall updated FELIX-3590:
-----------------------------------

    Component/s: Gogo Runtime
       Priority: Minor  (was: Major)
    
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Comment Edited] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13411971#comment-13411971 ] 

Christian Schneider edited comment on FELIX-3590 at 7/11/12 10:19 PM:
----------------------------------------------------------------------

The system properties can change in some cases but perhaps we can live with the fact that the change is not reflected. I will check with the other Karaf developers if copying would be enough. It definately would be the simplest option.
                
      was (Author: chris@die-schneider.net):
    The can change in some cases but perhaps we can live with the fact that the change is not reflected. I will check with the other Karaf developers if copying would be enough. It definately would be the simplest option.
                  
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13411971#comment-13411971 ] 

Christian Schneider commented on FELIX-3590:
--------------------------------------------

The can change in some cases but perhaps we can live with the fact that the change is not reflected. I will check with the other Karaf developers if copying would be enough. It definately would be the simplest option.
                
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (FELIX-3590) Add system property resolution to CommandSessionImpl

Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/FELIX-3590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13411964#comment-13411964 ] 

Richard S. Hall commented on FELIX-3590:
----------------------------------------

There is no rule or best practice that system properties shouldn't change and it is painful to debug such things when they do happen.

If you want to access system properties, then it seems like you should using System.getProperty() or hiding it behind some abstraction of your own. It doesn't make sense, to me, to expand Gogo's session properties to provide inter-session properties.

In your scenario, if system properties don't change, then what is the issue with copying them into the session properties?
                
> Add system property resolution to CommandSessionImpl
> ----------------------------------------------------
>
>                 Key: FELIX-3590
>                 URL: https://issues.apache.org/jira/browse/FELIX-3590
>             Project: Felix
>          Issue Type: Improvement
>          Components: Gogo Runtime
>    Affects Versions: gogo.runtime-0.10.0
>            Reporter: Christian Schneider
>            Priority: Minor
>             Fix For: gogo.runtime-0.12.0
>
>
> Currently we wrap the CommandProcessor and CommandShell in karaf to simply add the resolution of system properties.
> See org.apache.karaf.shell.console.impl.jline.Activator in the karaf console module.
> In the innermost code it is only the following we override in CommandSessionImpl.
>         public Object get(String name) {
>             Object val = super.get(name);
>             if (val == null) {
>                 val = System.getProperty(name);
>             }
>             return val;
>         }
> So I propose to add this system property resolution to gogo. This would allow us to remove all the wrapping code in karaf.
> If you are interested I can provide a patch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira