You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by jeme <gi...@git.apache.org> on 2017/07/22 12:16:04 UTC
[GitHub] lucenenet pull request #209: RFC: LUCENENET-565: Porting of Lucene.Net.Repli...
GitHub user jeme opened a pull request:
https://github.com/apache/lucenenet/pull/209
RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Commit is for Review with comments about original Java Source for assistance.
- [ ] RevisionFile.Size - Please change to `Length` to .NETify
- [ ] IReplicator & IRevision: Please leave the "I" prefix in the actual name of the interfaces, but change the names of the files back to the original "Replicator" and "Revision". It is easier to maintain if the files are in the same order as they are in Java.
- [ ] Please change IReadOnlyCollection, IReadOnlyDictionary, etc to ICollection, IDictionary, etc.
- [ ] Please rename (the class only, not the file) IndexInputInputStream to IndexInputStream (since in .NET InputStream is just a Stream).
- [ ] Please add the project.json and PROJECTNAME.project.json files to the projects (we use these for NuGet dependencies as well as for specifying NuGet package info). See the other projects for examples.
- [ ] Please arrange the code so it is in a similar order as it was in Java. In cases where you need to order it differently (such as de-nesting types), add a "LUCENENET specific" comment to indicate it diverges from Java.
- [ ] Please ensure it will compile and tests will pass on .NET Core (we use the Lucene.Net.Portable.sln file to do these tests).
- [ ] Replace BinaryFormatter with Json
- [ ] Use DataInputStream and DataOutputStream instead of DataInput and DataOutput.
Note: Missing .NET Core project files because of the current situation with .NET Core/Standard project files as discussed in https://issues.apache.org/jira/browse/LUCENENET-565
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jeme/lucenenet Lucene.Net.Replicator
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucenenet/pull/209.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 #209
----
commit 6da4dd20d45b152df0c0cf5f4e6c90a8024682e5
Author: jeme <je...@outlook.com>
Date: 2017-07-22T10:52:19Z
LUCENENET-565: Porting of Lucene Replicator - Commit is for Review with comments about original Java Source for assistance.
----
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Could there be Lucene.Net without Apache? I don't know. There must be some reason why no developer before me went off on their own and decided to pull it out from under Apache (and I don't think there is anything in the licensing preventing it).
Actually way back the first time I began to look into Lucene on .NET I did come across a few other attempts on just that, but I guess they died off in favor for this, and to some extend thanks for that because one of them attempted to use a RAW port (no .NET-fications) and then just build a wrapper around it, in theory a good idea if you could do fully automated porting, but in practice not viable at all I think...
Other solutions simply used a JNBridge and the Lucene Jar's... No idea how well that would work, again I think you would have the wrapper layer. Sounds awfully ineffective...
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Happy to contribute if that'd help at all.
There are definitely [a lot of things](https://github.com/apache/lucenenet/blob/master/CONTRIBUTING.md#other-ways-to-help) you could do to help and it would be much appreciated. Not just with `Lucene.Net.Replicator.AspNetCore` when Jens completes his end, but there are a lot of new development tasks I would like to do to make the API more palatable before calling this "released". It dawned on me that if 2 people create a folder with the same name (`src/dotnet/`) on 2 different branches, there won't be a conflict since Git doesn't track folders anyway. So that is not a blocker for doing this work.
As I previously mentioned, I think we should have an integration package for each major UI framework (both thick and thin client) so we can wire up the boilerplate stuff in "the normal" way for that framework. Certainly, creating packages like `Lucene.Net.AspNetCore` and `Lucene.Net.Owin` (or MVC?) are up for grabs, starting with creating a fluent way to setup an index writer and/or reader at application startup, and working out what other stuff is common boilerplate code that we could save others from having to research when setting it up. There should also be some UI components to plug in to make adding the most common search features to applications easier - I am being vague here, because this will take some discovery.
Also, the most common 3rd party NuGet package is a Document - POCO mapper. This is high on the list of things to create a built-in solution for that will ultimately find a place in every one of the UI framework modules. It seems to me that taking a good hard look at the MVC `ModelBinder` implementation would be a good place to start with this. We should make it work by default with conventions like `ModelBinder`, but be able to override the defaults using .NET Attributes (being that we have both a `TextField` and a `StringField` that could map to a string property, etc.). Do note there is a `LazyDocument` implementation in `Lucene.Net.Misc` that is read-only and we wouldn't want to block its use in any way.
Then of course there are the fun ones - UI integrations with Highlighter, Suggest, and Facet. At least the latter two may require some front-end JavaScript work for thin clients to create the customizable boilerplate implementations so they don't have to be re-invented by every consumer. Or, there may already be some great JS plugins we can use/recommend. Plus we will need similar integration components with thick clients with similar goals. I'd like to try to avoid the "experimental" JS frameworks that could evaporate tomorrow and use popular frameworks like JQuery and Bootstrap (or others that most people have heard of), but we should cater to the SPA crowd, too. It seems like this could be a very deep rabbit hole if we created a package for each SPA framework, but if we do one I am sure that others might contribute more for their framework of choice.
BTW - when dealing with the DI stuff, there are a couple of great articles [here](http://blog.ploeh.dk/2014/05/19/di-friendly-library/) and [here](http://blog.ploeh.dk/2014/05/19/di-friendly-framework/) which are helpful (plus he wrote a great book, Dependency Injection in .NET). I'd like to try to avoid using statics for this stuff if at all possible and allow for as much interop with DI containers as possible. Coming up with a good solution for a singleton IndexWriter (which is Disposable) for some of the older web frameworks when they are not using DI is a bit of a challenge.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by AndyPook <gi...@git.apache.org>.
Github user AndyPook commented on the issue:
https://github.com/apache/lucenenet/pull/209
All good :) In no way is any criticism intended. Was just looking at it
from a micro/code perspective.
The switch is just fine as is, the case blocks are small, maybe just
extract it out into another method to isolate it from the setup and
exception catch stuff which is always noisy. If there's a desire to plug
into other http frameworks then the IReplicatorReq/Res works well too
Re: other frameworks... I think that, as we're dealing with http rather
than the higher framework, that it more depends on how it's all
bootstrapped. So owin style or Global.asax.
I think that it would be a very similar approach for webapi2/mvc4/5 as they
both have a notion of an owin pipeline. Though the types are different I
think. Not sure DI is a blocker here, wiring the bits up seems straight
forward enough.
For non owin, "all that's needed" is someplace with the current HttpContext
to to call the ReplicatorService if the path StartsWith urlPrefix. So, an
IHttpHandler or from GlobalAsax.BeginRequest
As you've kept the api to Replicator abstract then adding other frameworks
ought to be ok.
Happy to contribute if that'd help at all.
On 8 August 2017 at 14:07, Shad Storhaug <no...@github.com> wrote:
> @AndyPook <https://github.com/andypook>
>
> Good point on the context parameter. After taking a closer look
> <https://github.com/apache/lucene-solr/blob/releases/lucene-solr/4.8.1/lucene/replicator/src/java/org/apache/lucene/replicator/http/HttpClientBase.java#L77-L78>,
> there is a path parameter on the client. Why they didn't name it context
> to match the server is beyond me, but it appears to be one and the same.
> So, effectively, we can get rid of that parameter and replace it with
> urlPrefix because they are the same thing. And because we have a prefix
> (and indeed it is not optional), we don't need a route constraint.
>
> Funny how long it is taking us to work this out since they admitted on
> that blog post that they built replicator in just 1 day. But they certainly
> could have documented it better in the API docs instead of falling back on
> a blog to serve as the documentation.
>
> Actually, the above code wasn't intended to be a finished product, but a
> demonstration of how to fit the pieces together. And making the pieces fit
> first (and tests pass) before refactoring is a good strategy. One decision
> that I will probably end up changing is to make the parameter
> IReadOnlyDictionary instead of IDictionary, since we don't want anyone
> tinkering with the contents of this singleton after application startup.
>
> While I agree with you that the switch case statement in
> ReplicationService seems to be screaming out for refactoring, I think
> Jens is also right on the money on this approach. Being that there needs to
> be some server-side interaction between the runtime code and HTTP listener,
> it is not possible to just wrap this up into a generic service that can be
> installed with an installer. In fact, we should build similar integration
> packages for MVC 5, WebApi2, and possibly even web forms. And if we are
> doing that, we should wait to see how we can create those integration
> packages before refactoring ReplicationService (assuming refactoring it
> is even in the cards - it may ultimately prove to be more useful to serve
> as the generic "servlet" piece that is missing from .NET). I think what
> Jens did with the IReplicationRequest and IReplicationResponse
> abstractions is a fine idea that will ultimately prove useful for plugging
> into each of these frameworks. And if ReplicationService is something the
> user never has to deal with anyway, maybe it doesn't make sense to change
> it as tempting as that might be. After all, if we refactor it, then we have
> to also refactor the tests.
>
> @jeme <https://github.com/jeme>
>
> I am not implying you have to do all of the work to integrate with each of
> those frameworks, just finishing AspNetCore is enough. Actually,
> integrating with the other frameworks will be a bit more tricky because the
> lack of default DI container, so I the most intuitive approach might be to
> register the dictionary statically on those.
>
> I have arrived at a decision on a folder structure for the .NET wrapper
> projects. We should put Lucene.Net.Replicator.AspNetCore into a new
> directory src/dotnet/. This will be the place where all of the projects
> go that we aren't specifically porting from Java, and eventually we can
> move the Lucene.Net.ICU and lucene-cli projects and their tests there,
> too. However, if you want to give this new project a permanent home, go
> ahead and create that folder as part of this PR.
>
> This will also serve as a "contrib" folder, but to me calling a project
> "contrib" makes it sound like it is unfinished or inferior quality. We
> expect that these packages will be highly polished with a more intuitive
> API than the Java ported code.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/lucenenet/pull/209#issuecomment-320950344>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAvrq_7oi_Bmir0DbB5HClGLqLo34yw9ks5sWF2PgaJpZM4OgKD6>
> .
>
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
Ok so I think I will give up on commenting on all of the above because there is to much. I will as a general note say be careful that you don't get ahead of your self.
I personally think it would be a bad idea to put to much of all that AspNetCore, Owin etc. into this particular project (project as Lucene in a whole, not each csproj file)...
Instead I would separate them out and give them meaningful project names... and give them their own release cycle... (Actually I think the Lucene.Net.Replicator.AspNetCore project would belong in one of those "child projects", but that is a bit complicated as it's currently needed for tests, it might be possible to write the tests differently without it or add a minimalistic implementation into test instead)
They could still be under "Lucene.Net" organisation wise, but I think there would be many advantages in allowing them to have their own repo and life...
This would focus this repository on the port of lucene (as direct as possible, but also to support these projects) and each project could have it's individual focus... (Lucene on AspNetCore for all AspNetCore integration on different levels, Lucene on Nancy for that etc.)...
Also, your already on the edge of making some decisions that would be bad for larger scale projects... E.g. making a Singleton LuceneWriter (registered in a IoC Container or not) wouldn't that limit you to only having a single index?... that would seem like an odd direction to consider within this context as the Replicator directly supports shards... (aka multiple indexes?)
It also seems that as things are right now you can't release each individual csproject on it's own from here?... (E.g. releasing just the Lucene.Net package and not the rest?)... If that is the case then I think that speaks even more for putting these more "platform" specific integrations into their own project/repo/release cycle so they are not bound by each other... So that these packages could be release on a much more rapid cycle than Lucene.Net it self.
There is ofc. many considerations around such "branching out", but from having done so quite a few time my self on our own projects, I can say that it is so much more convenient in the long run...
As a final note, I won't be able to find time to take the Lucene.Net.Replicator.AspNetCore project much further, my goal was to focus on Lucene.Net.Replicator it self and provide a meaningful port of that that could be integrated into AspNetCore (which we haven't even adopted yet), AspNet4x, Nancy or similar frameworks with relative ease...
Which path you take with Lucene.Net.Replicator.AspNetCore is up to you... But I don't feel right about building anything two involved for a framework/platform I don't even use and therefore don't know all the ins and outs of...
To me it was an example integration that could help me find points in the Lucene.Net.Replicator which might cause difficulty when integrating or straight out felt wrong and there are a few things about the ReplicatorService that annoys me in that regard, but there is a broader picture as the HttpReplicator aligns to that same theme and then there is your desire to stay compatible with Java versions, which is questionably because they choose to actually serialize Exception objects, if java clients expect any meaning of that then we are not compatible at all as all Exceptions would end up as "Unknown Exceptions" in a Java client... and that goes for the other way around as well...
I do have a small super simplistic and extremely crude DEMO AspNetCore app that indexes and publishes data and then a Console client that pulls the replicas which works. I am trying to use this to weed out annoyances.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> It sounds like you are not too familiar with dependency injection (which is why I am following this approach), and although you can certainly get by without it, it is definitely something every developer should know going forward. I was reluctant to read up on DI at first and wasn't sure that doing so would be worth the time, but ever since I read Dependency Injection in .NET, it has forever changed the way I write code. And out of dozens of books that I have read there are very few I can say have accomplished that. But as they say, you can lead a horse to water, but you can't make them drink - do as you will :).
I don't know how you would get that idea? I have been usiing IoC/DI for years, often but not always paired with a IoC Container.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
I generally don't like the middleware idea, at least not as a starting point because it doesn't feel like what we are adding sits at such a "general" level... It doesn't feel like across the board infrastructure in the same way as Mvc, Ioc etc.
Another thing is that we don't really hide much complexity for the developer but instead take away his freedom to decide when and where indexes etc. should be initialized or at least provide some pre-stage knowledge of it as each index would have to have a replicator configured, these should then be accessible from where he writes to the index has they have to be notified of changes...
So all in all, I would like to provide both flexibility and ease of use, but I would rather do the later first with the first one in mind so that can be added on top later (Or even by 3rd party libraries where people could try out different approaches, so we can find the best one)...
In other words, I won't block people to implement it as middle-ware if they wan't to, even though I personally feel that would be misplaced, on the contrary... But I don't feel like I have enough knowledge and experience to provide them with a good solution yet... So I aim for a more general one.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
Jens, thanks for the feedback.
While I generally agree with your points, we are unfortunately obligated to work within the confines of the Apache organization, which comes with its own set of conditions. For one, there is a [formal release process](http://www.apache.org/legal/release-policy.html) we need to abide by (including a vote process that takes a minimum of 3 days per release), and I am not sure what is involved in setting up another Apache repo (nor am I sure I really want to go down that rabbit hole).
That said, we could follow the [Lucene project's](https://github.com/apache/lucene-solr) example with their separate release cycles of Lucene and SOLR: put everything into one repo and use separate tag names for each product. At least they started out that way - now they are releasing both products at the same time.
It probably makes sense, though. If we have a release vote on both product A and product B and product B depends on product A, and product A doesn't pass the release vote, then product B will be stalled anyway.
Of course, there is an alternative - build these integration packages outside of Apache's umbrella. But I am not sure what Apache's policy is in this regard (anyone?). Given the current situation with Spatial4n and LuceneNetDemo being on Itamar's personal account and no ability to push to them without being given special permission (by Itamar), and the fact that most of the projects on NuGet that depend on Lucene.Net are either dead or dying, it feels like this idea could go wrong as well. Sure, it is feasible for a huge team like Microsoft to do this, but trying to pull this off with a small team probably isn't very realistic.
Not to mention, platform support. With the release of .NET Standard 2.0 looming, we are looking at a sweeping update that will hit every project. And what of .NET Standard 2.5 when that is eventually released? As much as Microsoft has promised by creating .NET Standard, we still have some branching between platforms we need to maintain, although it isn't as bad as if we were trying to use a portable library. Now, multiply that update by the number of repos that we would have to update (each with their own ever diverging build script)...and we have a huge pile of work.
But I will take this into consideration. Perhaps after a few releases that are synced up, we can put together a new template in TeamCity so each of these integration packages can have their own release cycle. But since the team of active PMC members is so small, finding the 3 votes necessary for every one of those separate releases might not be very realistic. Our last beta vote had exactly the minimum 3 votes that were required (including my own).
> Also, your already on the edge of making some decisions that would be bad for larger scale projects... E.g. making a Singleton LuceneWriter (registered in a IoC Container or not) wouldn't that limit you to only having a single index?... that would seem like an odd direction to consider within this context as the Replicator directly supports shards... (aka multiple indexes?)
Actually, I was planning to make an API to be able to register multiple IndexWriters or IndexReaders at application startup as singleton. Something along the lines of...
```
services.AddSearch().WithIndex(...).WithIndex(...)
```
But the fact remains that the recommendation is to use a singleton IndexWriter (or Reader) per index, meaning if we didn't provide the tools do it with a simple straightforward API, everyone would end up having to do the research how to do it and write the same boilerplate code. And a lot of them would assume they could create an IndexReader instance on each request (or register it with the wrong lifetime) and find out just how poorly that performs.
I need to explore exactly how this can work in a way that doesn't block people from doing it their own way, providing all of the overloads that allow pre-built implementations to be passed in as an alternative to using fluent builders. But regardless of how you slice it, setting up an index reader or writer belongs in the application startup phase and nowhere else, although you need to access those instances at runtime (again best solved with DI). Microsoft is providing a standard way for doing this in their latest UI frameworks and it would be a real shame not to take advantage of this fact.
> As a final note, I won't be able to find time to take the Lucene.Net.Replicator.AspNetCore project much further, my goal was to focus on Lucene.Net.Replicator it self and provide a meaningful port of that that could be integrated into AspNetCore (which we haven't even adopted yet), AspNet4x, Nancy or similar frameworks with relative ease...
> Which path you take with Lucene.Net.Replicator.AspNetCore is up to you... But I don't feel right about building anything two involved for a framework/platform I don't even use and therefore don't know all the ins and outs of...
No problem. Your contribution is much appreciated.
> To me it was an example integration that could help me find points in the Lucene.Net.Replicator which might cause difficulty when integrating or straight out felt wrong and there are a few things about the ReplicatorService that annoys me in that regard, but there is a broader picture as the HttpReplicator aligns to that same theme and then there is your desire to stay compatible with Java versions, which is questionably because they choose to actually serialize Exception objects, if java clients expect any meaning of that then we are not compatible at all as all Exceptions would end up as "Unknown Exceptions" in a Java client... and that goes for the other way around as well...
I see. You are right that it might not entirely interoperate with the Java version (at least not if there are exceptions), but at least we have explored that option and came to this conclusion.
> I do have a small super simplistic and extremely crude DEMO AspNetCore app that indexes and publishes data and then a Console client that pulls the replicas which works. I am trying to use this to weed out annoyances.
Cool. Just let me know when you are ready to hand over the reigns. I am still exploring whether I can make API docs part of the upcoming beta release.
I would also like to tinker with getting replicator working in a demo setup. Perhaps you can put the demo project in a separate repo? When you are ready, that is.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> I don't know how you would get that idea? I have been usiing IoC/DI for years, often but not always paired with a IoC Container.
My bad. To me it seems natural to want to put all of the startup code in the composition root (which Microsoft has made a nice way to do) and I made an assumption that one had to do with the other...It really is 3:45am, isn't it? :)
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
So for the past week I have been trying to make a "prototype" project that integrates this... all in all working with AspNetCore it self actually posed far more trouble than integrating the replicator into it as an afterthought... (Which I think is what will happen in many cases)...
It does have some gotchas and I am considering some changes in the HttpReplicator and HttpClientBase as a consequence... Even though the ReplicatorService it self was super easy to integrate, there is things I am reconsidering about it...
Here is an example of it integrated into a "Mvc"Controller:
```csharp
[Route("api/[controller]")]
public class ReplicateController : Controller
{
[HttpGet("{*path}")]
public void Get(string path) => Index.Instance.Service.Perform(Request, Response);
}
```
This requires the replicator to be created as:
```csharp
replicator = new LocalReplicator();
Service = new ReplicationService(new Dictionary<string, IReplicator>()
{
.... indexes
}, "/api/replicate");
```
Most notable is the `[Route("api/[controller]")]` + the controller type that has to be in sync with whats provided in the ReplicationService ctor: `"/api/replicate"` (replicate because we named the controller ReplicateController.
> I would expect there to be an extension method for IHostingEnvironment or IApplicationBuilder so it could be added to the application:
I don't think that would make all that much sense, there is allot more to it and I think it would be difficult, at least at this point to make the integration so simple as the "user" needs to manage publishing revisions etc. them self...
But I also don't think "middleware" is the right place for this, instead it depends on the kind of solution your making, often I would think that you would have some sort of web-api along side of it, in this case it should go into a controller, much like above however it DOES feel a bit awkward that we have a Get method that is void and doesn't have any parameters at all.
However I do think it fair to provide the signature currently given by the `Lucene.Net.Replicator.AspNetCore` as a basic approach as that is very similar to the Java version, which means it feels familiar and people will be able to read Java examples and somewhat translate them to .NET.
However I would like to supplement this with something that felt more natural, but it requires allot more thought, something that might be easier with a "working" solution in front of me, which I have now... But it would still take time.
So short term brings me back to that there are some minor reconsideration I wan't to take on the HttpReplicator and HttpClientBase as well as the ReplicatorService and the abstraction.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
Jens,
I have upgraded the project [here](https://github.com/NightOwl888/lucenenet/tree/replicator) to work on .NET Standard 1.5, cleaned up the doc comments, and fixed the member accessibility.
Currently, I am working on making a ReplicationHttpListener service that wraps TcpListener and IReplicationService (containing only the Perform method). I have a working prototype (HTTP 1.0), but am still cleaning up the rough edges. It is very basic, but allows you to pass in your own TcpListener so if someone were inclined to use HTTPS, authentication, client certificates, etc. it is possible to do. It is pretty clear we still need integration with AspNetCore and other thin clients since this will be incompatible with those, but this listener should handle the general case where someone wants to build a thick client to act as a replication server, and will work even on .NET Standard 1.5.
My hope is to use the ReplicationHttpListener for the main tests. We should migrate the AspNetCore specific tests to a new test project Lucene.Net.Tests.AspNetCore and build that out with the fluent API similar to what I showed before (although, it may make sense to move the reusable bits to the Support directory in Lucene.Net.Replicator). This will decouple the projects so at least your suggestion of moving Lucene.Net.AspNetCore to a separate repository is possible.
Anyway, if you want to refactor ReplicationService into separate concerns that would be okay as long as we keep the Perform method signature as it is.
As for upgrading to AspNetCore 2.0, it would probably be better to multi-target (1.0 and 2.0), since we will be adding support to run .NET Core 2.0 to the project soon (running tests on .NET Framework 4.5.1, .NET Core 1.0, and .NET Core 2.0).
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Rethink HttpClientBase and HttpReplicator
This gave a number of things to add.
- [ ] More constructor overloads (done)
- [ ] Async methods (not done, proved not as easy as I thought for some reason so I am holding back for now)
> Rethink ReplicatorService, abstractions and AspNetCore implementation.
This was the road I wen't down of when I started to refactor the ReplicatorService where I found that if we did so it no longer had much purpose, so this I would consider canceled...
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by AndyPook <gi...@git.apache.org>.
Github user AndyPook commented on the issue:
https://github.com/apache/lucenenet/pull/209
Making ```Perform``` async could be done like...
```csharp
public async Task PerformAsync(IReplicationRequest request, IReplicationResponse response)
{
...
try
{
switch (action)
{
case ReplicationAction.OBTAIN:
string sessionId = ExtractRequestParam(request, REPLICATE_SESSION_ID_PARAM);
string fileName = ExtractRequestParam(request, REPLICATE_FILENAME_PARAM);
string source = ExtractRequestParam(request, REPLICATE_SOURCE_PARAM);
using (Stream stream = replicator.ObtainFile(sessionId, source, fileName))
await stream.CopyToAsync(response.Body);
break;
case ReplicationAction.RELEASE:
replicator.Release(ExtractRequestParam(request, REPLICATE_SESSION_ID_PARAM));
break;
case ReplicationAction.UPDATE:
string currentVersion = request.QueryParam(REPLICATE_VERSION_PARAM);
SessionToken token = replicator.CheckForUpdate(currentVersion);
if (token == null)
{
await response.Body.WriteAsync(new byte[] { 0 }, 0, 1); // marker for null token
}
else
{
await response.Body.WriteAsync(new byte[] { 1 }, 0, 1);
token.Serialize(new DataOutputStream(response.Body));
}
break;
default:
throw new ArgumentOutOfRangeException();
}
}
...
}
```
Note ```CopyToAsync``` and ```WriteAsync```. It's a shame that DataOutputStream isn't async. A little out of scope for this though :)
On the trivia front. The ReplicatorBuilder fluent methods can just ```return this``` rather than newing up another builder each time.
Shouldn't the template building part be
```
string template = (string.IsNullOrWhiteSpace(urlPrefix) ? "" : urlPrefix) +
"/{context}/{shard}/{action}";
```
Otherwise the template would not start with "/" if urlPrefix was missing and it would throw?
Though I would suggest that urlPrefix should be mandatory. Otherwise it will steal all the requests from whatever comes next in the middleware pipeline.
Q: Why do you say it's "horrible to block the default route"?
This seems entirely natural to me :) Even back in asp you could add handlers that would intercept certain paths/extensions. It doesn't seem any different to Swagger intercepting "/swagger". Or OAuth intercepting it's control flow.
Lastly, an observation... My intent with using the template was that the Perform method would be refactored to accept the parts from the route. If you want to leave the Perform method as is (closer to java implementation I'm guessing) which parses the path and qs then the ```MapWhen``` approach might be better/simpler...
```csharp
public static void UseLuceneReplicator3(this IApplicationBuilder app, string urlPrefix, object indexService)
{
if (string.IsNullOrEmpty(urlPrefix) || !urlPrefix.StartsWith("/"))
throw new ArgumentException("urlPrefix MUST be provided");
var replicatorAccessor = app.ApplicationServices.GetService<IReplicatorAccessor>();
var replicationService = new ReplicationService(replicatorAccessor.Replicators);
app.MapWhen(context => context.Request.Path.StartsWithSegments(urlPrefix), appBuilder =>
{
appBuilder.Run(async context =>
{
await replicationService.PerformAsync(
new AspNetCoreReplicationRequest(context.Request),
new AspNetCoreReplicationResponse(context.Response));
});
});
}
```
As the accessor and service are singletons, they only need resolving once rather than every request.
Lastly, the template we've been looking at has a {context] segement, however the Perform method only parses 'shard' and 'action'. Is that significant?
Just stream of thought and observations :)
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
@jeme
I got a chance to get a deeper look of this structure, and I found a comment on that blog post that holds the answer to what I think is the part we are struggling with:
> The server can use ReplicationService to embed in a servlet which responds to HTTP requests sent by HttpReplicator [on the client side]. The server also uses LocalReplicator to manage the revisions. The indexing code on the server will call localReplicator.publish() and the servlet (through ReplicationService) will call localReplicator.checkForUpdate, obtain etc.
> The clients can use ReplicationClient, with an HttpReplicator, to replicate index changes from the server. The HttpReplicator is given the host:port of the server which manages the index revisions. The code examples above show how it can be done.
So, there are not 2 pieces to this, but 3:
1. A ReplicationService (on the server), which indeed does nothing more than listen for incoming requests. The files are replicated from the server to the client when it receives a command.
2. A LocalReplicator (on the server), which does the publishing of revisions.
3. An HTTPReplicator (on the client), which calls the ReplicationService with the commands. It receives the files from the server and updates files local to the client.
Since the ReplicationService (the HTTP listener) accepts an `IDictionary<string, IReplicator>` through its constructor, and we know that an HTTP listener must be registered at application startup, I think it is safe to assume that these replicators need to be registered as singletons (best to do it with DI, but I suppose a static would also work). If you look at the [HttpReplicatorTest](https://github.com/apache/lucene-solr/blob/releases/lucene-solr/4.8.1/lucene/replicator/src/test/org/apache/lucene/replicator/http/HttpReplicatorTest.java#L48), it registers this dependency at the class level (which essentially makes it singleton for the test).
So, first we need to devise a way to have multiple replicators as singleton:
```c#
services.AddFileReplication(r => r.WithShard("shard1").WithShard("shard2", new MyCustomReplicator()));
```
Using a fluent builder API, you can register multiple shards with "speak-able" syntax. Internally, this would just register a custom service that encapsulates a dictionary to hold the configuration data similar to the `IHttpContextAccessor`.
```c#
public interface IReplicatorAccessor
{
IDictionary<string, IReplicator> Replicators { get; }
IReplicator GetReplicator(string shard);
}
public class ReplicatorAccessor : IReplicatorAccessor
{
public ReplicatorAccessor(IDictionary<string, IReplicator> replicators)
{
this.Replicators = replicators ?? throw new ArgumentNullException(nameof(replicators));
}
public IDictionary<string, IReplicator> Replicators { get; private set; }
public IReplicator GetReplicator(string shard)
{
IReplicator result;
Replicators.TryGetValue(shard, out result);
return result;
}
}
```
This is wired up with an extension method:
```c#
public static class ServiceCollectionExtensions
{
public static void AddFileReplication(this IServiceCollection services, Func<ReplicatorBuilder, ReplicatorBuilder> expression)
{
if (services == null)
throw new ArgumentNullException(nameof(services));
if (expression == null)
throw new ArgumentNullException(nameof(expression));
var starter = new ReplicatorBuilder();
var builder = expression(starter);
AddFileReplication(services, builder.Build());
}
public static void AddFileReplication(this IServiceCollection services, IDictionary<string, IReplicator> replicators)
{
if (services == null)
throw new ArgumentNullException(nameof(services));
if (replicators == null)
throw new ArgumentNullException(nameof(replicators));
services.AddSingleton<IReplicatorAccessor>(new ReplicatorAccessor(replicators));
}
}
```
and the dictionary is built using a fluent builder. This is the most interesting part:
```c#
public class ReplicatorBuilder
{
private readonly IDictionary<string, IReplicator> replicators;
public ReplicatorBuilder()
: this(new Dictionary<string, IReplicator>())
{
}
public ReplicatorBuilder(IDictionary<string, IReplicator> replicators)
{
this.replicators = replicators ?? throw new ArgumentNullException("replicators");
}
public ReplicatorBuilder WithShard(string shard)
{
replicators.Add(shard, new LocalReplicator());
return new ReplicatorBuilder(replicators);
}
public ReplicatorBuilder WithShard(string shard, IReplicator replicator)
{
replicators.Add(shard, replicator);
return new ReplicatorBuilder(replicators);
}
public IDictionary<string, IReplicator> Build()
{
return replicators;
}
}
```
With that one line registered at application startup, you can ask for the `IReplicatorAccessor` by adding a constructor argument for it.
```c#
public class ValuesController : Controller
{
private readonly IReplicatorAccessor replicatorAccessor;
public ValuesController(IReplicatorAccessor replicatorAccessor)
{
this.replicatorAccessor = replicatorAccessor ?? throw new ArgumentNullException("replicatorAccessor");
}
```
And then you can use it to get the replicator instances by shard name.
```c#
[HttpGet]
public IEnumerable<string> Get()
{
IReplicator replicator = replicatorAccessor.GetReplicator("shard1");
replicator.Publish(new IndexRevision(writer));
return new string[] { "value1", "value2" };
}
```
Now, if the server needs to be registered as an HTTP listener, we just need another extension method to do that, similar to @AndyPook 's example. Basically, we register a route that runs our ReplicationService logic, which uses the `IReplicatorAccessor` to get the dictionary where they live and pass it to the `ReplicationService`.
```c#
public static class ApplicationBuilderExtensions
{
public static void UseFileReplication(this IApplicationBuilder app)
{
UseFileReplication(app, string.Empty);
}
public static void UseFileReplication(this IApplicationBuilder app, string urlPrefix)
{
var routeBuilder = new RouteBuilder(app);
string template = (string.IsNullOrWhiteSpace(urlPrefix) ? "" : urlPrefix + "/") +
"{context}/{shard}/{action}";
routeBuilder.MapGet(template, async context =>
{
var replicatorAccessor = app.ApplicationServices.GetService<IReplicatorAccessor>();
var replicationService = new ReplicationService(replicatorAccessor.Replicators);
replicationService.Perform(new AspNetCoreReplicationRequest(context.Request),
new AspNetCoreReplicationResponse(context.Response));
await Task.FromResult(0);
});
var routes = routeBuilder.Build();
app.UseRouter(routes);
}
}
```
So, in a nutshell you were right that there is more to the server than a simple HTTP listener. But I was right that the HTTP listener doesn't have any runtime behavior and thus needs to be registered at startup. So, we are both right :).
We just needed a way to register the shards in a way that they can be used both at startup and at runtime. But it looks like you were exactly on the right path where we need to go.
BTW - not sure about the `await` above. Maybe we should provide a way to `await` the `Perform()` method?
The above is horrible in terms of how it blocks the default MVC route - we need at least one route constrant. The simplest option (although not as thorough at it should be) would just be to make a constraint that matches the shard name against the keys in the `IReplicatorAccessor.Replicators.Keys` property. Suggestions on how to make it a bit more reliable than that welcome. Also, I wasn't able to work out how to use a constraint in conjunction with a handler...more time (or suggestions) needed.
The client can simply do its stuff by calling replicator API methods directly, there is no need to build anything special into AspNetCore for that.
As for the IndexWriters and IndexReaders, we can also provide similar extension methods to register them as singletons and access them in services (but not in `Lucene.Net.Replicator` - we'll build a package named `Lucene.Net.AspNetCore` for that).
So, if you don't mind putting this together like I have above and cleaning up the Java comments, I think this will be good enough to merge. Or if you don't have the time let me know and I will get it done.
There are some unfinished documentation comments and some methods that should be virtual (in Java they are virtual by default), but I think I can manage these. The important thing is that all of the test pass.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Actually way back the first time I began to look into Lucene on .NET I did come across a few other attempts on just that, but I guess they died off in favor for this, and to some extend thanks for that because one of them attempted to use a RAW port (no .NET-fications) and then just build a wrapper around it, in theory a good idea if you could do fully automated porting for every Lucene release, but in practice not viable at all I think...
> Other solutions simply used a JNBridge and the Lucene Jar's... No idea how well that would work, again I think you would have the wrapper layer. Sounds awfully ineffective...
It sounds like you are referring to what happens when you use [IKVM](http://www.ikvm.net/userguide/tutorial.html) to convert a `.jar` to MSIL byte code. You get a full-on Java-style API including the ridiculously long namespace names.
I recently have been looking into using it to avoid porting some of the Analysis dependencies (such as UIMA, which is quite large). I wouldn't use such a thing for Lucene.Net because of the performance impact it would have (which I have read is something like a 20-25% loss, but I suspect would be much higher than that). But even [Standford University](https://sergey-tihon.github.io/Stanford.NLP.NET/) is using IKVM-processed Java for natural language processing, so I think this would be adequate for (at least index time) analysis.
Unfortunately, even that (apparently long-standing) project has [recently died out](http://weblog.ikvm.net/default.aspx). There will never be .NET Standard support for IKVM unless someone picks up the torch and keeps moving it forward.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> While I generally agree with your points, we are unfortunately obligated to work within the confines of the Apache organization, which comes with its own set of conditions. For one, there is a formal release process we need to abide by (including a vote process that takes a minimum of 3 days per release), and I am not sure what is involved in setting up another Apache repo (nor am I sure I really want to go down that rabbit hole).
Holy crap... I am glad I am not under that umbrella, in an age of continuous delivery and continuous deployment that sounds like poison... o.O...
> That said, we could follow the Lucene project's example with their separate release cycles of Lucene and SOLR: put everything into one repo and use separate tag names for each product. At least they started out that way - now they are releasing both products at the same time.
> It probably makes sense, though. If we have a release vote on both product A and product B and product B depends on product A, and product A doesn't pass the release vote, then product B will be stalled anyway.
Product B would always depend on a Released A. (Could be a Beta Release or Final Release, but it would be something that was out there)... It would work the same way as if I choose to create a package that used Lucene.NET...
> Of course, there is an alternative - build these integration packages outside of Apache's umbrella. But I am not sure what Apache's policy is in this regard (anyone?). Given the current situation with Spatial4n and LuceneNetDemo being on Itamar's personal account and no ability to push to them without being given special permission (by Itamar), and the fact that most of the projects on NuGet that depend on Lucene.Net are either dead or dying, it feels like this idea could go wrong as well. Sure, it is feasible for a huge team like Microsoft to do this, but trying to pull this off with a small team probably isn't very realistic.
I doubt very much that putting things in under ASF is what will keep it alive, actually I would think the opposite as ASF seems like such a scary thing to get started with which probably scares off allot of contributors... Besides, I think the thriving world of OpenSource proves that even single person contributions can end up becoming projects with huge communities... But ofc. you can't just put something into OpenSource and then think it will just take a life of it's own... It requires effort... But again...
I would say that a more likely cause for the many deaths of Lucene.Net dependent projects was that Lucene.Net it self seemed to have died. I have even questioned my use of Lucene.Net in favor for Solr/ELK due to the version gap...
> But the fact remains that the recommendation is to use a singleton IndexWriter (or Reader) per index, meaning if we didn't provide the tools do it with a simple straightforward API, everyone would end up having to do the research how to do it and write the same boilerplate code. And a lot of them would assume they could create an IndexReader instance on each request (or register it with the wrong lifetime) and find out just how poorly that performs.
Thats not a singleton though... without knowing for sure, I am fairly sure that it's not just a recommendation in this context, firstly the writer needs to be configured with a specific deletion policy and you create revisions based on the writer, so having multiple writers for a single index sounds like something that would break... I am not sure though, I could be wrong...
So what your talking about is having a registry of indexes (with writers and readers)... That registry could be registered as a singleton... (Sounds much like my LuceneIndexContext on a different project >.<)
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Repli...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/lucenenet/pull/209
---
[GitHub] lucenenet issue #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Another thing is that we don't really hide much complexity for the developer but instead take away his freedom to decide when and where indexes etc. should be initialized or at least provide some pre-stage knowledge of it as each index would have to have a replicator configured, these should then be accessible from where he writes to the index has they have to be notified of changes...
I disagree. We are not taking anything away from the user by doing this - they are always free to drop to the lower-level Lucene APIs to do any low-level configuration such features.
But the grand plan is (hopefully...someday...with enough contributions) to have integration packages with all of the .NET UI frameworks so application-level configuration can be done at application startup using DI and so all of the boilerplate code that everyone writes to add search to the application can be reduced to a fluent API configuration (the same way that most .NET libraries do it). We know nearly every web app will need an IndexWriter per index registered as a singleton. Why not do that with one (or two) `.AddSearch()` method where the configuration operations are specified as simple variables in one place (the place where every other global feature of the app is configured)? We could easily provide a way to do most of the common options that apply to 80-90% of users (and would save 80-90% of users from having to deal with a complex API to get the simple features they want).
I don't see any reason at all to expose the HttpRequest/HttpResponse in this API - there are only a few ways this can go that are sensible. Sure, this might make sense at a low level (particularly if this is the piece that we plug into every HTTP listener-capable framework), but these details are not necessary for it to be useful for the end user (after all, they are interacting with it through a URL).
And sure, there are other features of Lucene that need to interact with the UI directly and so forth that need other integration APIs to interoperate. But certainly there is no question that an HTTP listener is a one-time per application thing, not something that would ever be registered in a controller. The client on the other hand may be a different story.
Also, we should make the .NET version of replicator support interop with the Java Lucene replicator. For that to work, the URL scheme should be the same as Lucene by default. The [replication service documentation](https://lucene.apache.org/core/4_8_0/replicator/org/apache/lucene/replicator/http/ReplicationService.html) clearly specifies this as:
```
/<context>/<shard>/<action>
```
Of course, we should probably provide a way to override this - routing conflicts happen. But this is the logical default setting. I haven't looked into whether this is even configurable in Java or would require a custom compile in order to get it to interoperate.
Most likely the common use case for the server will be a standalone application that serves as a server only. So, I would expect routing conflicts in this situation to be rare.
Even if you go the path of using a controller for this (which is an option), we should stay away from attribute routing for the simple reason that it is impossible to change after it is compiled into the library. A better argument for never using it is the fact that routing is *order sensitive* and .NET Reflection (which is how Attributes are read) by definition has *undefined order*. I have answered who knows how many questions on StackOverflow for people who have hit that landmine. The solution is to add an Order parameter to the attributes, but once again if the attribute is compiled into the DLL there is no way to fix this problem.
On the other hand, using convention-based routing allows you do define the AddLuceneReplication() method where the route will be added in relation to the other routes, based on the order the methods are called at application startup (which is unclear to me if that is possible with middleware - it should be...).
I made a similar implementation in MvcSiteMapProvider for the [`XmlSiteMapController`](https://github.com/maartenba/MvcSiteMapProvider/blob/master/src/MvcSiteMapProvider/MvcSiteMapProvider/Web/Mvc/XmlSiteMapController.cs). It registers its own routes using convention-based routing. It used WebActivator to load the routing for the controller. Although, this was before Microsoft made the nice `Startup.cs` class where everything could be configured using extension methods so this is not exactly how I would do it now. Instead, I would give the user the ability to configure it in `Startup.cs`, where other 3rd parties do it. And that is where I would provide extension method overloads to configure alternate route URLs and any other advanced options that may be needed.
The user of course always has the option to *not* call the method at application startup, build their own controller, and dig into any of the more advanced options (assuming there are any left that are not in the extension method overloads). But why should everyone have to do this?
> it's apparently not that simple, and currently I don't fully get the idea behind the design from the java implementation... The things I do get though is that it is a master/slave implementation and a polling implementation.
There is [some documentation](https://github.com/apache/lucene-solr/blob/releases/lucene-solr/4.8.0/lucene/replicator/src/java/org/apache/lucene/replicator/package.html) in the repo about how replicator is configured in Java, but it seems to be missing from the [4.8.0 API docs](https://lucene.apache.org/core/4_8_0/replicator/index.html).
A quick search also reveals [this mailing list thread](https://lists.gt.net/lucene/java-user/225677) which links to [a blog post](http://shaierera.blogspot.com/2013/05/the-replicator.html) that seems to describe it in more detail.
Does this help? If not, I suggest contacting the Lucene team (and link to the above) via the Lucene user list to see if they can provide better answers on the intended workflow. I skimmed it, but it is still not very clear to me where the client (who calls the server every 30 seconds) would need to be. Maybe a Quartz.net task? Or a Windows service/other type of persistent app that runs when the computer is started? Certainly, that is the part that has the most complexity - it needs to keep track of the URLs to call to replicate everything and provide the commands to the replication servers.
> Move EnumrableExtensions to Lucene.Support? or Lucene.Util?
Lucene.Support is the only place where we are putting code that is not a port of something from Java, so that would be the place to put it.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
@AndyPook
Good point on the `context` parameter. After [taking a closer look](https://github.com/apache/lucene-solr/blob/releases/lucene-solr/4.8.1/lucene/replicator/src/java/org/apache/lucene/replicator/http/HttpClientBase.java#L77-L78), there is a `path` parameter on the client. Why they didn't name it `context` to match the server is beyond me, but it appears to be one and the same. So, effectively, we can get rid of that parameter and replace it with `urlPrefix` because they are the same thing. And because we have a prefix (and indeed it is not optional), we don't need a route constraint.
Funny how long it is taking us to work this out since they admitted on that blog post that they built replicator in just 1 day. But they certainly could have documented it better in the API docs instead of falling back on a blog to serve as the documentation.
Actually, the above code wasn't intended to be a finished product, but a demonstration of how to fit the pieces together. And making the pieces fit first (and tests pass) before refactoring is a good strategy. One decision that I will probably end up changing is to make the parameter `IReadOnlyDictionary` instead of `IDictionary`, since we don't want anyone tinkering with the contents of this singleton after application startup.
While I agree with you that the switch case statement in `ReplicationService` seems to be screaming out for refactoring, I think Jens is also right on the money on this approach. Being that there needs to be some server-side interaction between the runtime code and HTTP listener, it is not possible to just wrap this up into a generic service that can be installed with an installer. In fact, we should build similar integration packages for MVC 5, WebApi2, and possibly even web forms. And if we are doing that, we should wait to see how we can create those integration packages before refactoring `ReplicationService` (assuming refactoring it is even in the cards - it may ultimately prove to be more useful to serve as the generic "servlet" piece that is missing from .NET). I think what Jens did with the `IReplicationRequest` and `IReplicationResponse` abstractions is a fine idea that will ultimately prove useful for plugging into each of these frameworks. And if `ReplicationService` i
s something the user never has to deal with anyway, maybe it doesn't make sense to change it as tempting as that might be. After all, if we refactor it, then we have to also refactor the tests.
@jeme
I am not implying you have to do all of the work to integrate with each of those frameworks, just finishing AspNetCore is enough. Actually, integrating with the other frameworks will be a bit more tricky because the lack of default DI container, so I the most intuitive approach might be to register the dictionary statically on those.
I have arrived at a decision on a folder structure for the .NET wrapper projects. We should put `Lucene.Net.Replicator.AspNetCore` into a new directory `src/dotnet/`. This will be the place where all of the projects go that we aren't specifically porting from Java, and eventually we can move the `Lucene.Net.ICU` and `lucene-cli` projects and their tests there, too. However, if you want to give this new project a permanent home, go ahead and create that folder as part of this PR.
This will also serve as a "contrib" folder, but to me calling a project "contrib" makes it sound like it is unfinished or inferior quality. We expect that these packages will be highly polished with a more intuitive API than the Java ported code.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by AndyPook <gi...@git.apache.org>.
Github user AndyPook commented on the issue:
https://github.com/apache/lucenenet/pull/209
just playing... here's how a middleware might look
```csharp
public class Startup
{
// usual boiler plate
// add some method that bootstraps the index and replication service
public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
loggerFactory.AddConsole(Configuration.GetSection("Logging"));
loggerFactory.AddDebug();
app.UseLuceneReplicator("/_replicator", "replicaService123");
app.UseMvc();
}
}
// these bits live in Lucene.Net.Replicator.AspNetCore
public static class LuceneReplicatorMiddlewareExtensions
{
public static IApplicationBuilder UseLuceneReplicator(this IApplicationBuilder builder, PathString prefix, object replicaService)
{
return builder.Map(prefix, app => app.UseMiddleware<LuceneReplicatorMiddleware>(replicaService));
}
}
public class LuceneReplicatorMiddleware
{
private readonly RequestDelegate next;
private readonly object replicaService;
public LuceneReplicatorMiddleware(RequestDelegate next, object replicaService)
{
this.next = next;
this.replicaService= replicaService;
}
public async Task Invoke(HttpContext context)
{
var req = context.Request;
await context.Response.WriteAsync($"<p>{DateTime.Now.ToString()}</p>");
await context.Response.WriteAsync($"<p>replicaService={replicaService.ToString()}</p>");
await context.Response.WriteAsync($"<p>path={req.Path}</p>");
await context.Response.WriteAsync($"<ul>");
foreach(var q in req.Query)
await context.Response.WriteAsync($"<li>{q.Key}={q.Value}</li>");
await context.Response.WriteAsync($"</ul>");
}
}
```
```app.Map``` intercepts requests with that prefix (they call it "branching the request pipeline")
Where I've used "replicaService123" is just a demo of how to pass stuff into the middleware. You'd substitute the ```ReplicationService```. Then ```Invoke``` could be as simple as ```replicaService.Perform(context.Request, context.Response)```. But it might be better to pull the exception handling part out into the middleware and just pass the response stream into the service.
Note: ```req.Path``` is just the part coming after the prefix
Anyway, just an idea
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
Hi Jens,
I'd like to get this merged so I can start working on upgrading to the new `.csproj` format and adding support for the [now released .NET Standard 2.0](https://github.com/dotnet/announcements/issues/24), and a fresh beta release of Lucene.Net.
What more needs to happen with this to get it ready?
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Holy crap... I am glad I am not under that umbrella, in an age of continuous delivery and continuous deployment that sounds like poison... o.O...
It's both a blessing and a curse. For example, we would never be able to pull this off without the resources of the CI account (TeamCity) that we have setup. It takes at least an hour to run the tests on each platform, so we need something we can run them in parallel on for at least that long. I tried this with MyGet and it turns out there is a limit to 15 minutes per build. So, having a full CI server with huge amounts of storage and RAM is a good thing. I also got some help from the team setting it up.
Also, Apache requires that all code must be testable before being released (implying there must be tests), and ensure that the licensing is all legal, all code files have license headers, etc.
But not being able to setup your own repo (because they are hosted by Apache) and having to deal with JIRA instead of just using GitHub's issue tracker causes some friction.
Could there be Lucene.Net without Apache? I don't know. There must be some reason why no developer before me went off on their own and decided to pull it out from under Apache (and I don't think there is anything in the licensing preventing it).
But we do what we can. Recently, there have been more contributions than since I started working on this a year ago. I am not sure if that is because we are now so close to the finish line, or my efforts of updating documentation, getting a release on NuGet, etc are starting to pay off.
But I think that to help combat this we really need to update the documentation, website, simplify deployment, etc. and generally make Lucene.Net a good place to work. Key among these things is to lower the bar for how difficult it is to add search to an application, and that is what I am hoping to achieve with these integration packages (which in turn should increase both downloads and contributions). But of course, if you would rather use the low-level APIs directly they are not going anywhere.
> So what your talking about is having a registry of indexes (with writers and readers)... That registry could be registered as a singleton... (Sounds much like my LuceneIndexContext on a different project >.<)
Yes, it is probably similar. Although it will probably end up being an `IIndexAccessor` or something along those lines.
It sounds like you are not too familiar with dependency injection (which is why I am following this approach), and although you can certainly get by without it, it is definitely something every developer should know going forward. I was reluctant to read up on DI at first and wasn't sure that doing so would be worth the time, but ever since I read [Dependency Injection in .NET](https://www.manning.com/books/dependency-injection-in-dot-net), it has forever changed the way I write code. And out of dozens of books that I have read there are very few I can say have accomplished that. But as they say, you can lead a horse to water, but you can't make them drink - do as you will :).
> Anyways, this is derailing things a bit, it was just some input for you going forward. So we should probably get back to focusing on the actual Lucene.Net.Replicator, and comments in there >.<...
> Ill see if I can find a suitable place for that little demo app.
Thanks again. Replicate away! And if you can, find a way to replicate contributors :).
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> > Not sure exactly what you mean by this. But, you might consider doing a local build build -pv:4.8.0-beta00005, and then you can reference the NuGet package locally so you can work with it as if it were a release.
> Y, that is what I am doing now but it means that if I am to share the project (e.g. on github) then I have to check those in or leave instructions for the "ugly workaround"... when the next beta release is there it's just nuget packages and all dependencies is pulled in as one would expect.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
Funny you mentioned that. I was struggling to get the ReplicationHttpListener to work with the HttpClient due to the async weirdness that is going on there. I even made an attempt to use the HttpListener for .NET Framework-only support, but that didn't pan out either. I ended up postponing that piece for now so I could work on the `.csproj` migration and get that out of the way.
Speaking of which, I am considering excluding the Lucene.Net.Replicator.AspNetCore project from the CI build (and the release) until it is more complete. Would there be a point to releasing the Lucene.Net.Replicator package without it? I am just wondering whether it is required for the functionality to work or if you are providing enough of a guide to get it working without that piece.
We can always work on it more after the next beta release, I am just wondering if it should be in the release and if so how much of it? But it is simpler to deal with if it is in master because of the sweeping changes that need to be done.
Unfortunately, after I installed the latest version of VS 2017 and .NET Core 2.0 SDK on my machine, it broke all ability to test Lucene.Net on .NET Core - and it is simpler to upgrade to the fully supported `.csproj` format than to spend the time to work out why this old preview that Microsoft doesn't support anymore doesn't work. I just have a little more to do before pushing this to master and trying out the new build process on TeamCity.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
Hi Jens,
Before I give this a proper review, I would like to try to plug this into an AspNetCore app to see if I can get it to work. Could you provide some instructions to how to plug this in?
I would expect there to be an extension method for `IHostingEnvironment` or `IApplicationBuilder` so it could be added to the application:
```c#
public class Startup
{
public Startup(IHostingEnvironment env)
{
var builder = new ConfigurationBuilder()
.SetBasePath(env.ContentRootPath)
.AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
.AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true)
.AddEnvironmentVariables();
Configuration = builder.Build();
}
public IConfigurationRoot Configuration { get; }
// This method gets called by the runtime. Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services)
{
// Add framework services.
services.AddMvc();
}
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
loggerFactory.AddConsole(Configuration.GetSection("Logging"));
loggerFactory.AddDebug();
app.UseMvc();
}
}
```
For example `services.AddLuceneReplicator()` or `app.UseLuceneReplicator()`. I would also expect it to configure the default routing to act as a server, since out of the box it should use the same URL scheme as Lucene's replicator.
Maybe what we have here is a lower level bare-bones component which we will also need, but it feels like the additional steps to get from here to a working app need to be simplified (or at the very least documented).
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> Not sure exactly what you mean by this. But, you might consider doing a local build build -pv:4.8.0-beta00005, and then you can reference the NuGet package locally so you can work with it as if it were a release.
Y, that is what I am doing now but it means that if I am to share the project (e.g. on github) then I have to check those in or leave instructions for the "ugly workaround"... when the next beta release is there it's just nuget packages and all dependencies is pulled in as one would expect.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
Seems you misunderstood me a little bit :)...
If your fixing documentation comments and adding virtual modifiers, then I don't think I have anything else to add to the code, then it's all documentation from here on.
> As for upgrading to AspNetCore 2.0, it would probably be better to multi-target (1.0 and 2.0), since we will be adding support to run .NET Core 2.0 to the project soon (running tests on .NET Framework 4.5.1, .NET Core 1.0, and .NET Core 2.0).
It was only for the Sample. I won't dive that deep for a sample TBH... It's just meant to demonstrate how one could integrate it, nothing more... (Code wise)... And then use that as input to the missing documentation (similar to the blog post)...
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
FYI - I have started cleaning up the documentation comments already so I can merge this. Just wanted you to know so we aren't duplicating efforts. Thanks again for the contribution.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by AndyPook <gi...@git.apache.org>.
Github user AndyPook commented on the issue:
https://github.com/apache/lucenenet/pull/209
Just sticking my 2p/2c in again...
Having this as middleware with a ```UseLuceneReplicator("rootPrefix", service)``` would be a lot more "natural" from an aspnetcore point of view.
Part of the reason that the controller looks a little odd is due to the signature of ```Perform```. It is odd to use the Request/Response properties in a Controller. In order to make the routing work it also needs "{path*}" and the arg etc which is ignored cos it actually pulls those direct from the Request.
Looking at how Perform works, middleware taking control of the req/res at the owin layer without all the mvc/routing shenanigans. Having it intercept a few path(s) (ie ```StartsWith(routprefix```) would seem the "right thing to do". That way the developer can just add the middleware without needing to get involved in creating a "weird" controller or messing with routing or thinking about DI or...
The MVC feature is just middleware after all.
I think there may be an odd side effect if an exception occurs part way through. ie a partial response with a json serialized exception added to the end which would be hard for the client to deal with (although if this is how the java version works...).
Or if you think the Controller approach is better, I could imagine wrapping some of the behavior in a custom ```IActionResult```. This could deal with handling the exception and creating the right kind of http response. This would leave the ReplicationService with just creating the stream.
Might also be worth digging into how aspnetcore deals with big streams (ie does it buffer them in memory before sending the first byte like old asp/iis did or can it do something smarter).
Anyway, just some thoughts... what do you think?
I ought to put my money where my mouth is, write some code and contribute :)
I haven't used VS2015 in what seems like forever. We'll see what the weekend brings
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
I had started to see if we could refactor the replicator service in a way that made it more flexible towards routing parameters etc... But I quickly realized that then there was not much reason to have the service anymore as it mostly just became a glorified dictionary over the shards... So I don't think that made much sense.
So it's there to allow some quick integrations small scale integrations, but on a bigger scale I don't really see it begin all that useful, but that is actually also a topic they touch in the blog post so I guess that's fair for now...
I have been really busy this week so I have not had all that much time but I am working on making on transforming my crude little example into something a bit more meaningful as well as upgrading to AspNetCore 2.0 on the example >.<...
That would provide some "example" documentation here, but it would actually be a bit easier to share the demo project ones this PR is in and the release is out as I don't have to pull that in in a dirty way.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by AndyPook <gi...@git.apache.org>.
Github user AndyPook commented on the issue:
https://github.com/apache/lucenenet/pull/209
@jeme you are quite right, that is equivalent.
A bit OCD, but I have a tendency to try and encapsulate features in discrete pieces. If this got much more complex that a one liner then I think splitting it out into a separate class would be a "good thing", easier to create tests for...
I agree with @NightOwl888. It seems that this "api" is very well defined and opinionated (esp if you want interop) so leaving this to an end developer to reimplement each time would be error prone or cut&paste boiler plate.
In short, keeping this well defined service api outside of mvc world with it's attributes, reflection, media type formatters etc. would also be a good thing
Although I may be missing some nuances and be off in the weeds somewhere.
If the path is defined as ```/<context>/<shard>/<action>``` then maybe using routing would be helpful. Maybe something like...
```csharp
public static void UseLuceneReplicator2(this IApplicationBuilder app, string prefix, object indexService)
{
var routeBuilder = new RouteBuilder(app);
routeBuilder.MapGet(prefix+ "/{context}/{shard}/{action}/{*path}", async context =>
{
var req = context.Request;
await context.Response.WriteAsync($"<p>{DateTime.Now.ToString()}</p>");
await context.Response.WriteAsync($"<p>indexService={indexService.ToString()}</p>");
await context.Response.WriteAsync($"<p>path={req.Path}</p>");
await context.Response.WriteAsync($"<p>context={context.GetRouteValue("context")}</p>");
await context.Response.WriteAsync($"<p>shard={context.GetRouteValue("shard")}</p>");
await context.Response.WriteAsync($"<p>action={context.GetRouteValue("action")}</p>");
await context.Response.WriteAsync($"<ul>");
foreach (var q in req.Query)
await context.Response.WriteAsync($"<li>{q.Key}={q.Value}</li>");
await context.Response.WriteAsync($"</ul>");
});
var routes = routeBuilder.Build();
app.UseRouter(routes);
}
```
Again I'd suggest taking the lamdba body out into something more discrete, this is just a demo.
Just throwing stuff at the wall to see what sticks :)
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by jeme <gi...@git.apache.org>.
Github user jeme commented on the issue:
https://github.com/apache/lucenenet/pull/209
> (or at the very least documented)
This though, will very much be needed because as mentioned, it's apparently not that simple, and currently I don't fully get the idea behind the design from the java implementation... The things I do get though is that it is a master/slave implementation and a polling implementation.
Right now I am trying to see if I can find some equivalent documentation from the Java parts.
---
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 #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/209
> If your fixing documentation comments and adding virtual modifiers, then I don't think I have anything else to add to the code, then it's all documentation from here on.
Well, there are...
- Rethink HttpClientBase and HttpReplicator
- Rethink ReplicatorService, abstractions and AspNetCore implementation.
But I think those should probably wait until we have integrations with MVC, WebApi, and web forms so we can ensure the pieces all fit together right. I still think you did an excellent job of finding the lowest common denominator between any type of HTTP listener, and I doubt we will need to refactor the request/response part of this, but there may still be some work to do with these two classes.
> > As for upgrading to AspNetCore 2.0, it would probably be better to multi-target (1.0 and 2.0), since we will be adding support to run .NET Core 2.0 to the project soon (running tests on .NET Framework 4.5.1, .NET Core 1.0, and .NET Core 2.0).
> It was only for the Sample. I won't dive that deep for a sample TBH... It's just meant to demonstrate how one could integrate it, nothing more... (Code wise)... And then use that as input to the missing documentation (similar to the blog post)...
Ahh, okay.
> That would provide some "example" documentation here, but it would actually be a bit easier to share the demo project ones this PR is in and the release is out as I don't have to pull that in in a dirty way.
Not sure exactly what you mean by this. But, you might consider doing a [local build](https://github.com/NightOwl888/lucenenet/tree/replicator#building-and-testing) `build -pv:4.8.0-beta00005`, and then you can reference the NuGet package locally so you can work with it as if it were a release.
Or, if you mean you need to link to something in the repo, let me know and I will see about finalizing the locations of everything and merging to master to facilitate your documentation work.
---
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.
---