Skip to content
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

Saved objects export - unable to export when references are missing #43876

Closed
legrego opened this issue Aug 23, 2019 · 9 comments · Fixed by #47685
Closed

Saved objects export - unable to export when references are missing #43876

legrego opened this issue Aug 23, 2019 · 9 comments · Fixed by #47685
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@legrego
Copy link
Member

legrego commented Aug 23, 2019

The Saved Objects Export API (and corresponding UI feature) can optionally retrieve all references when exporting saved objects.

If the requested object references a non-existent object, the export operation will fail altogether.

I think it would be better if the export succeeded, or at least allowed for partial success.

curl 'http://localhost:5601/api/saved_objects/_export' -H 'kbn-version: 8.0.0' -H 'Content-Type: application/json' --data-binary '{"objects":[{"id":"722b74f0-b882-11e8-a6d9-e546fe2bba5f","type":"dashboard"}],"includeReferencesDeep":true}'

{"statusCode":400,"error":"Bad Request","message":"Bad Request","attributes":{"objects":[{"id":"3ba638e0-b894-11e8-a6d9-e546fe2bba5f","type":"search","error":{"statusCode":404,"message":"Not found"}},{"id":"2c9c1f60-1909-11e9-919b-ffe5949a18d2","type":"map","error":{"statusCode":404,"message":"Not found"}}]}}```
@legrego legrego added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Saved Objects labels Aug 23, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@rudolf
Copy link
Contributor

rudolf commented Aug 26, 2019

We had a short discussion in a related bug #42343 (which really was two bugs in one, so I'm glad you opened this issue)

I agree that warning about missing references but still exporting as much as possible seems to me like the most desirable behaviour.

Since the API returns the export NDJSON we can't "inline" an error message. One way to work around that would be to have a ignoreMissingReferences option in the API. The UI could then try to perform an export and if that fails because of missing references, display a warning dialog and retry with ignoreMissingReferences = true.

@rudolf rudolf added the Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages label Aug 26, 2019
@rudolf
Copy link
Contributor

rudolf commented Aug 26, 2019

Based on #43233 I think we should also show which objects include the missing references so users are able to delete those objects or manually fix the references.

@legrego
Copy link
Member Author

legrego commented Aug 26, 2019

Since the API returns the export NDJSON we can't "inline" an error message. One way to work around that would be to have a ignoreMissingReferences option in the API. The UI could then try to perform an export and if that fails because of missing references, display a warning dialog and retry with ignoreMissingReferences = true.

Based on #43233 I think we should also show which objects include the missing references so users are able to delete those objects or manually fix the references.

This approach makes sense to me!

@alexfrancoeur
Copy link

Ran into the same issue here: #45520. Maybe the default experience should disable the toggle or set to off if a user is exporting all objects?

@joshdover joshdover added the bug Fixes for quality problems that affect the customer experience label Sep 18, 2019
@joshdover
Copy link
Contributor

We prioritized this in our 7.5 roadmap, but we're going to timebox to 1 week. If solving this takes longer, we should re-evaluate as a team before sinking more time into it.

@pgayvallet
Copy link
Contributor

Adding the ignoreMissingReferences option makes a lot of sense for the API.

Main question is, how do we integrate this from the savedObjects page in the best way. The NDJSON format really limits what we can do in term of edge cases and user informations in case of successful call, as we have no way to add additional data (such as missing references and referencing objects) to the response without breaking the format. In case of error however, we can add anything to the error payload to actually inform the user.

Options I see here without impacting the NDJSON format:

  • A) Identify from front-end an export failure caused by missing references, as we include them in the error payload, and perform a second call as Rudolf suggest (call with ignoreMissingReferences=false, then if it fails because of non-existing references, call again with ignoreMissingReferences=true)
    pro: as we are performing both calls in sequence, we are able, if first call fails because of missing references and second call succeed, to actually inform the user of the missing references from first call when second call succeed.
    con: more complex to implement that other options.

  • B) Simply add another toggle in the UI to let the user choose if the ignoreMissingReferences option should be true or false when performing exports.
    pro: the user is at least aware if there are missing references when exporting with the option set to false
    con: as we are not performing both calls in the same handler, we can't inform the user of missing references as we can in option A)

  • C) Simply always call the API with ignoreMissingReferences=true
    pro: simplest
    con: user will have no way to know if there was missing references and which ones.

I guess option A) is the best one in term of user experience.

@rudolf do you see anything else ?

Other point, do we agree that even with ignoreMissingReferences=true, the call should fails if requesting to export non-existing objects (and not missing references to the objects requested) ?

I.E

curl 'http://localhost:5601/api/saved_objects/_export' -H 'kbn-version: 8.0.0' -H 
'Content-Type: application/json' --data-binary '{"objects":[{"id":"722b74f0-b882-
11e8-a6d9-e546fe2bba5f","type":"dashboard"}],"includeReferencesDeep":
true, "ignoreMissingReferences": true}'

with {"id":"722b74f0-b882-11e8-a6d9-e546fe2bba5f","type":"dashboard"} not present in the index should still fail

@pgayvallet
Copy link
Contributor

pgayvallet commented Oct 9, 2019

I though we could touch the NDJSON output for retro compat (importing in older than exported), but we are actually not supporting this, so after discussing with Rudolf, the solution we came to is the following:

  • The export API will never fails in case of missing references
  • At the end of the NDJSON output, we will append a new line/object, containing the export metadatas:
    • input query for the export
    • number of exported objects
    • number and identifiers of missing references
    • (probably not now) the exported objects -> missing references mapping to help users identifying / cleaning up potential missing references
  • The import API will be modified to detect/ignore this metadata entry during import
  • (if needed) we can add an option to the export endpoint to not include this metadata entry in the output, such as excludeExportDetails

@legrego not sure of your actual usage of the API. Are you using the import endpoint or are you importing back using another mecanism ? Is this proposal acceptable to your needs ?

@kobelb
Copy link
Contributor

kobelb commented Oct 9, 2019

@pgayvallet the Copy to Space API uses SavedObjectsLegacyService::importExport internally. It first calls getSortedObjectsForExport and then uses importSavedObjects. As long as the changes which are made are compatible with our usages, I don't anticipate any issues: https://github.com/elastic/kibana/blob/85c8232c0b81e9e0dd2265e04b2cc26dffaf1b60/x-pack/legacy/plugins/spaces/server/lib/copy_to_spaces/copy_to_spaces.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants