You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by conniey <gi...@git.apache.org> on 2016/08/29 02:52:50 UTC
[GitHub] lucenenet pull request #183: Fix test failures and making SPIClassIterator m...
GitHub user conniey opened a pull request:
https://github.com/apache/lucenenet/pull/183
Fix test failures and making SPIClassIterator more performant
### SPIClassIterator
* Filtering out Microsoft/System assemblies to improve loading
* Only adding public classes to the iterator:
* Fixed test failures where MockCodec (which had a codec name "Lucene46") was being loaded instead of Lucene46Codec because it was being found first in the iterator
* Consolidated assembly loading code
### Test Failure Fixes
* Adding a default ctor to the Codecs that were not being found because SPIClassIterator was skipping over them
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/conniey/lucenenet fixTestFailures
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucenenet/pull/183.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 #183
----
commit 461a59c03a0c23d088d382ea3b0ddaa640427ced
Author: Connie Yau <co...@microsoft.com>
Date: 2016-08-28T23:06:10Z
Fixing test failures by adding default ctor in Codec classes
commit 6f406aa7a9f7a5b05832214b89edd247122d4cf1
Author: Connie Yau <co...@microsoft.com>
Date: 2016-08-28T23:27:32Z
Making MockCodecs private so they are not loaded in SPIClassIterator
commit 0f9228eb7996d1989f30ed38d101389ccbb322c2
Author: Connie Yau <co...@microsoft.com>
Date: 2016-08-29T02:47:46Z
Only loading non-Microsoft Assemblies to search for public types
----
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/183
@conniey
Thanks.
It occurred to me that the AnalysisSPILoader only uses classes with a constructor that take a single parameter of type `IDictionary<string, string>`. So, as long as we're optimizing, perhaps the SPIClassIterator could be limited to default constructors and constructors with one parameter of `IDictionary<string, string>`. That should eliminate a lot of types from being considered.
I did a quick search and the NamedSPILoader and AnalysisSPILoader are the only 2 classes that use the SPIClassIterator in lucene, so it should be safe to do this.
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:
https://github.com/apache/lucenenet/pull/183
After merging #173 this branch has merge conflicts, @conniey can you resolve please?
---
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] lucenenet pull request #183: Fix test failures and making SPIClassIterator m...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/lucenenet/pull/183
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/183
@conniey - I ran this against Analysis.Common and it doesn't break any tests. As @synhershko mentioned though, it has some merge conflicts.
The latest commit [here](https://github.com/NightOwl888/lucenenet/tree/analysis-bugz-2) resolves them.
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by conniey <gi...@git.apache.org>.
Github user conniey commented on the issue:
https://github.com/apache/lucenenet/pull/183
@NightOwl888 I took a look at your branch and merged your `NamedSPILoader` and `SPIClassIterator<S>` changes to avoid you having to do any fix-ups with those classes.
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by conniey <gi...@git.apache.org>.
Github user conniey commented on the issue:
https://github.com/apache/lucenenet/pull/183
@NightOwl888 I updated the [iterator to filter](https://github.com/conniey/lucenenet/blob/5611f70455199a97e3b7180300d356612f07a97e/src/Lucene.Net.Core/Util/SPIClassIterator.cs#L90-L108) for types with default constructors or `IDictionary<string,string>`. Is this OK now?
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:
https://github.com/apache/lucenenet/pull/183
Looks good. Pending @NightOwl888 approval before merging this in. Thanks @conniey !
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by conniey <gi...@git.apache.org>.
Github user conniey commented on the issue:
https://github.com/apache/lucenenet/pull/183
@synhershko The merge conflicts are resolved now. :)
---
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] lucenenet issue #183: Fix test failures and making SPIClassIterator more per...
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/183
Thanks. Do note that Analysis.Common also depends on SPIClassIterator, and I had to fix it so it would load classes that don't have a default constructor (which is what the Analysis.Util.AnalysisSPILoader requires). I also modified the NamedSPIClassLoader to filter out the classes without a default constructor.
However, the changes still haven't been merged into master from the [analysis-work branch](https://github.com/apache/lucenenet/tree/analysis-work). Could you please verify these changes work with Analysis.Common and also that they won't cause a merge conflict when it is merged into master?
---
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.
---