You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Ramkumar R. Aiyengar" <an...@gmail.com> on 2015/04/09 09:09:11 UTC
Two argument Paths.get
May be we should forbid calls to the two argument version of Paths.get.
Especially given other wishes as well apart from Hdfs (like to use a
different filesystem for tests), this call, which uses the default
filesystem, seems to be a mistake waiting to happen..
On 9 Apr 2015 07:48, "Varun Thacker (JIRA)" <ji...@apache.org> wrote:
>
> [
> https://issues.apache.org/jira/browse/SOLR-6637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486843#comment-14486843
> ]
>
> Varun Thacker commented on SOLR-6637:
> -------------------------------------
>
> Oh that looks like my mistake. Sorry about that.
>
> We need to change this {{tmpIndex = Paths.get(solrCore.getDataDir(),
> tmpIdxDirName).toString();}} to
> {{Paths.get(solrCore.getDataDir()).resolve(tmpIdxDirName).toString();}}
>
> There were two more places which need to be corrected. I'll fix them.
>
> > Solr should have a way to restore a core
> > ----------------------------------------
> >
> > Key: SOLR-6637
> > URL: https://issues.apache.org/jira/browse/SOLR-6637
> > Project: Solr
> > Issue Type: Improvement
> > Reporter: Varun Thacker
> > Assignee: Varun Thacker
> > Fix For: Trunk, 5.2
> >
> > Attachments: SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch
> >
> >
> > We have a core backup command which backs up the index. We should have a
> restore command too.
> > This would restore any named snapshots created by the replication
> handlers backup command.
> > While working on this patch right now I realized that during backup we
> only backup the index. Should we backup the conf files also? Any thoughts?
> I could separate Jira for this.
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>
Re: Two argument Paths.get
Posted by "Ramkumar R. Aiyengar" <an...@gmail.com>.
Yeah, looks like there is a
Paths.get(String, String...)
and a
Paths.get(URI)
The thing I was looking to forbid was Paths.get with varargs being
non-empty, there's no way to do that as far as I can see.
The alternative is to explicitly use FileSystem.getDefault() in places
where the first is needed with varargs empty and ban the call altogether.
That's a lot of places, may be we can put a wrapper around it, but might be
still worthwhile to at least make it clear that this is expected to take
only a local disk filename.
On 9 Apr 2015 10:51, "Varun Thacker" <va...@gmail.com> wrote:
> I think it could be a good idea to add it to the forbidden-apis check. I'm
> unsure if we can do that though, since its just one method.
>
> Also tests failed but were nighties so didn't get caught while committing
> unfortunately and if we move tests to another filesystem then tests should
> fail as well. So maybe not having an explicit check for it is not that bad?
>
> On Thu, Apr 9, 2015 at 12:43 PM, Ramkumar R. Aiyengar <
> andyetitmoves@gmail.com> wrote:
>
>> I meant the *multiple* arg Paths.get.. (its a vararg function)
>> On 9 Apr 2015 08:09, "Ramkumar R. Aiyengar" <an...@gmail.com>
>> wrote:
>>
>>> May be we should forbid calls to the two argument version of Paths.get.
>>> Especially given other wishes as well apart from Hdfs (like to use a
>>> different filesystem for tests), this call, which uses the default
>>> filesystem, seems to be a mistake waiting to happen..
>>> On 9 Apr 2015 07:48, "Varun Thacker (JIRA)" <ji...@apache.org> wrote:
>>>
>>>>
>>>> [
>>>> https://issues.apache.org/jira/browse/SOLR-6637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486843#comment-14486843
>>>> ]
>>>>
>>>> Varun Thacker commented on SOLR-6637:
>>>> -------------------------------------
>>>>
>>>> Oh that looks like my mistake. Sorry about that.
>>>>
>>>> We need to change this {{tmpIndex = Paths.get(solrCore.getDataDir(),
>>>> tmpIdxDirName).toString();}} to
>>>> {{Paths.get(solrCore.getDataDir()).resolve(tmpIdxDirName).toString();}}
>>>>
>>>> There were two more places which need to be corrected. I'll fix them.
>>>>
>>>> > Solr should have a way to restore a core
>>>> > ----------------------------------------
>>>> >
>>>> > Key: SOLR-6637
>>>> > URL: https://issues.apache.org/jira/browse/SOLR-6637
>>>> > Project: Solr
>>>> > Issue Type: Improvement
>>>> > Reporter: Varun Thacker
>>>> > Assignee: Varun Thacker
>>>> > Fix For: Trunk, 5.2
>>>> >
>>>> > Attachments: SOLR-6637.patch, SOLR-6637.patch,
>>>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
>>>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch
>>>> >
>>>> >
>>>> > We have a core backup command which backs up the index. We should
>>>> have a restore command too.
>>>> > This would restore any named snapshots created by the replication
>>>> handlers backup command.
>>>> > While working on this patch right now I realized that during backup
>>>> we only backup the index. Should we backup the conf files also? Any
>>>> thoughts? I could separate Jira for this.
>>>>
>>>>
>>>>
>>>> --
>>>> This message was sent by Atlassian JIRA
>>>> (v6.3.4#6332)
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>>>> For additional commands, e-mail: dev-help@lucene.apache.org
>>>>
>>>>
>
>
> --
>
>
> Regards,
> Varun Thacker
>
Re: Two argument Paths.get
Posted by Shalin Shekhar Mangar <sh...@gmail.com>.
On Thu, Apr 9, 2015 at 7:40 PM, Shawn Heisey <ap...@elyograg.org> wrote:
> On 4/9/2015 3:50 AM, Varun Thacker wrote:
> > I think it could be a good idea to add it to the forbidden-apis check.
> > I'm unsure if we can do that though, since its just one method.
>
> We can forbid specific versions of an overloaded method. In the String
> class, toLowerCase() and toUpperCase() are forbidden, but the signatures
> toLowerCase(Locale) and toUpperCase(Locale) are allowed.
>
> One thing I do not know for sure is whether forbidden-apis allows a
> forbidden signature that includes a variable number of arguments. I
> looked at some of the included signatures and couldn't tell.
>
It does. A var-arg is just an array e.g. I recently added
java.text.MessageFormat#format(java.lang.String,java.lang.Object[]) to
forbidden-apis.
>
> Thanks,
> Shawn
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>
--
Regards,
Shalin Shekhar Mangar.
Re: Two argument Paths.get
Posted by Shawn Heisey <ap...@elyograg.org>.
On 4/9/2015 3:50 AM, Varun Thacker wrote:
> I think it could be a good idea to add it to the forbidden-apis check.
> I'm unsure if we can do that though, since its just one method.
We can forbid specific versions of an overloaded method. In the String
class, toLowerCase() and toUpperCase() are forbidden, but the signatures
toLowerCase(Locale) and toUpperCase(Locale) are allowed.
One thing I do not know for sure is whether forbidden-apis allows a
forbidden signature that includes a variable number of arguments. I
looked at some of the included signatures and couldn't tell.
Thanks,
Shawn
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: Two argument Paths.get
Posted by Varun Thacker <va...@gmail.com>.
I think it could be a good idea to add it to the forbidden-apis check. I'm
unsure if we can do that though, since its just one method.
Also tests failed but were nighties so didn't get caught while committing
unfortunately and if we move tests to another filesystem then tests should
fail as well. So maybe not having an explicit check for it is not that bad?
On Thu, Apr 9, 2015 at 12:43 PM, Ramkumar R. Aiyengar <
andyetitmoves@gmail.com> wrote:
> I meant the *multiple* arg Paths.get.. (its a vararg function)
> On 9 Apr 2015 08:09, "Ramkumar R. Aiyengar" <an...@gmail.com>
> wrote:
>
>> May be we should forbid calls to the two argument version of Paths.get.
>> Especially given other wishes as well apart from Hdfs (like to use a
>> different filesystem for tests), this call, which uses the default
>> filesystem, seems to be a mistake waiting to happen..
>> On 9 Apr 2015 07:48, "Varun Thacker (JIRA)" <ji...@apache.org> wrote:
>>
>>>
>>> [
>>> https://issues.apache.org/jira/browse/SOLR-6637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486843#comment-14486843
>>> ]
>>>
>>> Varun Thacker commented on SOLR-6637:
>>> -------------------------------------
>>>
>>> Oh that looks like my mistake. Sorry about that.
>>>
>>> We need to change this {{tmpIndex = Paths.get(solrCore.getDataDir(),
>>> tmpIdxDirName).toString();}} to
>>> {{Paths.get(solrCore.getDataDir()).resolve(tmpIdxDirName).toString();}}
>>>
>>> There were two more places which need to be corrected. I'll fix them.
>>>
>>> > Solr should have a way to restore a core
>>> > ----------------------------------------
>>> >
>>> > Key: SOLR-6637
>>> > URL: https://issues.apache.org/jira/browse/SOLR-6637
>>> > Project: Solr
>>> > Issue Type: Improvement
>>> > Reporter: Varun Thacker
>>> > Assignee: Varun Thacker
>>> > Fix For: Trunk, 5.2
>>> >
>>> > Attachments: SOLR-6637.patch, SOLR-6637.patch,
>>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
>>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch
>>> >
>>> >
>>> > We have a core backup command which backs up the index. We should have
>>> a restore command too.
>>> > This would restore any named snapshots created by the replication
>>> handlers backup command.
>>> > While working on this patch right now I realized that during backup we
>>> only backup the index. Should we backup the conf files also? Any thoughts?
>>> I could separate Jira for this.
>>>
>>>
>>>
>>> --
>>> This message was sent by Atlassian JIRA
>>> (v6.3.4#6332)
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>>> For additional commands, e-mail: dev-help@lucene.apache.org
>>>
>>>
--
Regards,
Varun Thacker
Re: Two argument Paths.get
Posted by "Ramkumar R. Aiyengar" <an...@gmail.com>.
I meant the *multiple* arg Paths.get.. (its a vararg function)
On 9 Apr 2015 08:09, "Ramkumar R. Aiyengar" <an...@gmail.com> wrote:
> May be we should forbid calls to the two argument version of Paths.get.
> Especially given other wishes as well apart from Hdfs (like to use a
> different filesystem for tests), this call, which uses the default
> filesystem, seems to be a mistake waiting to happen..
> On 9 Apr 2015 07:48, "Varun Thacker (JIRA)" <ji...@apache.org> wrote:
>
>>
>> [
>> https://issues.apache.org/jira/browse/SOLR-6637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486843#comment-14486843
>> ]
>>
>> Varun Thacker commented on SOLR-6637:
>> -------------------------------------
>>
>> Oh that looks like my mistake. Sorry about that.
>>
>> We need to change this {{tmpIndex = Paths.get(solrCore.getDataDir(),
>> tmpIdxDirName).toString();}} to
>> {{Paths.get(solrCore.getDataDir()).resolve(tmpIdxDirName).toString();}}
>>
>> There were two more places which need to be corrected. I'll fix them.
>>
>> > Solr should have a way to restore a core
>> > ----------------------------------------
>> >
>> > Key: SOLR-6637
>> > URL: https://issues.apache.org/jira/browse/SOLR-6637
>> > Project: Solr
>> > Issue Type: Improvement
>> > Reporter: Varun Thacker
>> > Assignee: Varun Thacker
>> > Fix For: Trunk, 5.2
>> >
>> > Attachments: SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch,
>> SOLR-6637.patch, SOLR-6637.patch, SOLR-6637.patch
>> >
>> >
>> > We have a core backup command which backs up the index. We should have
>> a restore command too.
>> > This would restore any named snapshots created by the replication
>> handlers backup command.
>> > While working on this patch right now I realized that during backup we
>> only backup the index. Should we backup the conf files also? Any thoughts?
>> I could separate Jira for this.
>>
>>
>>
>> --
>> This message was sent by Atlassian JIRA
>> (v6.3.4#6332)
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>>