-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[POC] Implented working config with multiple indexes per document class #917
base: 7.0
Are you sure you want to change the base?
Conversation
c18c166
to
c56878a
Compare
Codecov Report
@@ Coverage Diff @@
## 7.0 #917 +/- ##
============================================
+ Coverage 83.16% 84.15% +0.99%
- Complexity 413 425 +12
============================================
Files 39 39
Lines 1360 1395 +35
============================================
+ Hits 1131 1174 +43
+ Misses 229 221 -8
Continue to review full report at Codecov.
|
@saimaz Can I have an opinion on the proposed changes? I am not even sure this goes far enough. I think the new approach by using the class name as service name is generally flawed and you should rethink this design decision. |
The current architecture does not support multiple indexes for the same document. I'm fully aware that the current approach is not seamless how OOP law directs. The main idea was to simplify as much as possible to work with the document as the index. For me, personally, the current approach is very handy. Having said that I'm very open for suggestions, PR's or anything else that might help. |
Thank you for your response. Ok, with the approach I've implemented both will be possible. Can you review it? I'll create separate PR with my idea for the container approach later. |
Sure, I'l take a look |
@saimaz Any idea when you will have a chance to look at this PR? |
Hi @saimaz. I just wanted to ask again if there is a chance to have this PR reviewed? Or if you don't want to allow the same document used in multiple services? |
I've made a proof of concept for a solution where we can have multiple indexes for one document class again (#902). The config can look like this:
If a
class
parameter is provided theindexes
children keys can be a self defined string which will be used as the index service name (ifpackage_de_chf
is used, the service will be calledongr.es.index.package_de_chf
).I abused the
IndexSettings
class to create a temporary storage before creating the definition. I am not sure if this is a good solution or if we should just work with a separate class (only difference would be thepropertyMetadata
).I've also removed the return values from the setters since those are not used anywhere.
This PR can probably be targeted to the 6.2 branch too.