lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From NightOwl888 <...@git.apache.org>
Subject [GitHub] lucenenet issue #209: RFC: LUCENENET-565: Porting of Lucene.Net.Replicator
Date Tue, 08 Aug 2017 13:07:27 GMT
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.
---

Mime
View raw message