-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[ES] Move DB model out of v1/elasticsearch/internal/spanstore/internal
#6894
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6894 +/- ##
==========================================
- Coverage 96.12% 96.11% -0.02%
==========================================
Files 339 339
Lines 19597 19597
==========================================
- Hits 18838 18835 -3
- Misses 574 576 +2
- Partials 185 186 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@yurishkuro @mahadzaryab1 What should be my next PR, implementing the writer for v2 or a directory structure with reader/writer implemented with |
Unit tests are passing locally, have to see what's going wrong in CI |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
@yurishkuro @mahadzaryab1 Please review this PR as it is a blocker for implementing |
@@ -17,6 +17,7 @@ import ( | |||
"github.com/stretchr/testify/require" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spanstore/internal/dbmodel is unnecessary nesting. Move it directly to spanstore/
Signed-off-by: Manik2708 <[email protected]>
internal/storage/v1/elasticsearch/spanstore/dbmodel/from_domain_test.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Manik2708 <[email protected]>
…al` (jaegertracing#6894) ## Which problem is this PR solving? Fixes a part of jaegertracing#6458 ## Description of the changes - Currently `dbmodel` can't be accessed by the `v2`, therefore it is moved out of `v1/elasticsearch/internal/spanstore/internal` ## How was this change tested? - Unit Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Manik2708 <[email protected]>
…al` (jaegertracing#6894) ## Which problem is this PR solving? Fixes a part of jaegertracing#6458 ## Description of the changes - Currently `dbmodel` can't be accessed by the `v2`, therefore it is moved out of `v1/elasticsearch/internal/spanstore/internal` ## How was this change tested? - Unit Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Manik2708 <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
Which problem is this PR solving?
Fixes a part of #6458
Description of the changes
dbmodel
can't be accessed by thev2
, therefore it is moved out ofv1/elasticsearch/internal/spanstore/internal
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:npm run lint
andnpm run test