[ML] New Platform server shim: update usage collector to use core savedObjects #58058
Conversation
alvarezmelissa87
added
:ml
Feature:NP Migration
release_note:skip
|
Pinging @elastic/ml-ui (:ml) |
| )) as MlTelemetrySavedObject; | ||
| const mlTelemetrySavedObject = ((await savedObjects | ||
| .createInternalRepository() | ||
| .get('ml-telemetry', ML_TELEMETRY_DOC_ID)) as unknown) as MlTelemetrySavedObject; |
There was a problem hiding this comment.
Can you explain why the as unknown is now necessary? I wonder if this upcoming PR related to saved objects might allow us to do this in a better way? Is it worth waiting until it is in maybe?
There was a problem hiding this comment.
You should already be able to specify a generic for .get(), #58022, just makes it required. So .get<MlTelemetrySavedObject>('ml-telemetry', ML_TELEMETRY_DOC_ID) should work out of the box.
There was a problem hiding this comment.
Updated type in 486fc3b5cc351c937f217ab85fc9adc9fadf91a4
| @@ -69,6 +67,7 @@ export interface MlCoreSetup { | |||
| injectUiAppVars: (id: string, callback: () => {}) => any; | |||
| http: MlHttpServiceSetup; | |||
| savedObjects: SavedObjectsLegacyService; | |||
| coreSavedObjects: SavedObjectsServiceStart; | |||
There was a problem hiding this comment.
You might save yourself some time by naming this the same as it is on CoreStart:
| coreSavedObjects: SavedObjectsServiceStart; | |
| savedObjects: SavedObjectsServiceStart; |
There was a problem hiding this comment.
I see that you're using that name already for the legacy interface. I recommend switching that to a separate "legacy" object that gets passed into start, similar to the second code example in this section (look for LegacySetup): https://github.com/elastic/kibana/blob/master/src/core/MIGRATION.md#introduce-new-plugin-definition-shim
There was a problem hiding this comment.
Thanks, @joshdover. 😄 I think at this point, we're close enough to just cut over fully to NP so instead of making this change now I'll go ahead and switch over to NP in a follow-up PR
alvarezmelissa87
force-pushed
the
ml-np-usage-collector
branch
from
7f80996 to
86b5084
Compare
alvarezmelissa87
force-pushed
the
ml-np-usage-collector
branch
from
1857467 to
9464b57
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…edObjects (elastic#58058) * use NP savedObjects and createInternalRepository for usage collection * fix route doc typo * update MlTelemetrySavedObject type * remove fileDataVisualizer routes dependency on legacy es plugin * update mlTelemetry tests * remove deprecated use of getSavedObjectsClient
…edObjects (#58058) (#58367) * use NP savedObjects and createInternalRepository for usage collection * fix route doc typo * update MlTelemetrySavedObject type * remove fileDataVisualizer routes dependency on legacy es plugin * update mlTelemetry tests * remove deprecated use of getSavedObjectsClient
Summary
Related meta issue: #49743
Update usageCollector to use NP savedObjects and remove legacy deps.
Checklist
Delete any items that are not applicable to this PR.