You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by "Henri Biestro (JIRA)" <ji...@apache.org> on 2008/08/25 10:29:44 UTC
[jira] Created: (SOLR-724) CoreDescriptor.{get,set}CoreExpressions
should probably not be public (but package private)
CoreDescriptor.{get,set}CoreExpressions should probably not be public (but package private)
-------------------------------------------------------------------------------------------
Key: SOLR-724
URL: https://issues.apache.org/jira/browse/SOLR-724
Project: Solr
Issue Type: Bug
Affects Versions: 1.3
Reporter: Henri Biestro
Priority: Minor
Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
Since a 'public' method can not be removed easily, this is a future problem.
Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Shalin Shekhar Mangar resolved SOLR-724.
----------------------------------------
Resolution: Fixed
Fix Version/s: 1.3
Assignee: Shalin Shekhar Mangar
Committed revision 688751.
Bowing down to public demand :-)
Changing the methods to package private to insulate the public API from internal changes.
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Assignee: Shalin Shekhar Mangar
> Priority: Minor
> Fix For: 1.3
>
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625377#action_12625377 ]
Henri Biestro commented on SOLR-724:
------------------------------------
Obviously different :-)
In my mind, these are resources of the core-container / cores; resources are accessed through their respective ResourceLoader.
And the CoreDescriptor is, at its name hints, a parameter; it should not play any active role besides carrying data to create a SolrCore (and should not be used beyond that role).
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625346#action_12625346 ]
Henri Biestro commented on SOLR-724:
------------------------------------
When a method is public, it also has to be maintained & up-compatible; if you write a component that uses some private methods or members, that's your prerogative but there is no guarantee from Apache that it's existence and behavior will remain in the future.
In this issue, I'm stating that exposing Properties in the CoreDescriptor is hindering future evolution because they will have to be maintained, that the same information should be available from what I consider a narrower scope location that does not reduce the functional possibilities.
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625369#action_12625369 ]
Shalin Shekhar Mangar commented on SOLR-724:
--------------------------------------------
I feel that CoreContainer and CoreDescriptor seem to be the most logical place to *find* the properties since they directly correspond to the <solr> and <core> elements in solr.xml and SolrResourceLoader is the most logical place to *use* them since that's what we use to load config files. I think it should be OK to keep both the ways.
Thoughts?
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Mark Miller (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625390#action_12625390 ]
Mark Miller commented on SOLR-724:
----------------------------------
I agree with Henri.
Having these methods public ties core properties to the CoreDescriptor for no apparent good reason. This has the possibility of being an annoyance down the road. Unless specifically needed, why create this burden? Keeping the link between core properties and the descriptor package private allows for more flexibility in the future - and will keep users happier - we don't want to have to deprecate and change user exposed APIs if we can help it.
bq. My understanding was that till a use-case exists, we shouldn't publicly expose the method ( and it's been quite a hard lesson... )
I think this is a great rule of thumb, and should be the guiding principal here. Whats the use case that this solves beyond what using the resource loader provides? The benefit should clearly outweigh having to maintain/support the public API.
bq. Another question to ask before making things public is that can the user can alter some state in a way that can affect the functionality. If it is purely a read-only state I guess it should be OK
I think it goes further than this - there is a public link between the descriptor and the core properties. Its not a huge issue obviously, but there should be good reason for publicly linking two classes I think - the fact that the implementation contains the linked class privately doesn't seem like a very good reason.
- Mark
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724)
CoreDescriptor.{get,set}CoreExpressions should probably not be public (but
package private)
Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625277#action_12625277 ]
Shalin Shekhar Mangar commented on SOLR-724:
--------------------------------------------
I think you mean get/set coreProperties.
I don't think there's a problem. These methods can continue to be used for properties (evaluated expressions). We can always keep another object for un-evaluated expressions for persisting them back.
> CoreDescriptor.{get,set}CoreExpressions should probably not be public (but package private)
> -------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625319#action_12625319 ]
Noble Paul commented on SOLR-724:
---------------------------------
bq.Besides, the SolrResourceLoader is available from everywhere.
But , if there is something private in that class it should be fine . right?
bq.My understanding was that till a use-case exists..
Because Solr is also an API there are a lot of requesthandlers which people write may find some info useful. We can go one step ahead and think "will it hurt if this is public?" . Moreover , even if something is private I can easily access the value through reflection. So the kind of security it offers is just not real
The only advantage I see is that , if you wish to remove something it is easy if something is not public. So if some feature is experimental it can be kept not public
Another question to ask before making things public is that can the user can alter some state in a way that can affect the functionality. If it is purely a read-only state I guess it should be OK
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724)
CoreDescriptor.{get,set}CoreExpressions should probably not be public (but
package private)
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625287#action_12625287 ]
Henri Biestro commented on SOLR-724:
------------------------------------
Yes, correct; properties, not expressions.
They are unnecessary since the CoreContainer & each SolrCore resource loader could (will hopefully) contain the evaluated properties.
That would make 2 places where we can access them & this seems redundant.
I'm just trying to reduce public method exposure.
> CoreDescriptor.{get,set}CoreExpressions should probably not be public (but package private)
> -------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625305#action_12625305 ]
Henri Biestro commented on SOLR-724:
------------------------------------
I'm confused about when the 'what-if' rule can apply or not wrt public methods.
My understanding was that till a use-case exists, we shouldn't publicly expose the method ( and it's been quite a hard lesson... :-) )
Besides, the SolrResourceLoader *is* available from everywhere.
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated SOLR-724:
-------------------------------
Summary: CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private) (was: CoreDescriptor.{get,set}CoreExpressions should probably not be public (but package private))
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-724) CoreDescriptor.{get,set}CoreProperties
should probably not be public (but package private)
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625289#action_12625289 ]
Noble Paul commented on SOLR-724:
---------------------------------
bq.I'm just trying to reduce public method exposure.
There is no harm is exposing methods as long as we are exposing immutable stuff. It can actually enable a lot of components to get information w/o raising new issues in JIRA
> CoreDescriptor.{get,set}CoreProperties should probably not be public (but package private)
> ------------------------------------------------------------------------------------------
>
> Key: SOLR-724
> URL: https://issues.apache.org/jira/browse/SOLR-724
> Project: Solr
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Priority: Minor
>
> Exposing them precludes being ever able to fill the CoreDescriptor with property expressions.
> Since a 'public' method can not be removed easily, this is a future problem.
> Besides, as is, their is no reason for them to be public.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.