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

[Fleet] Reevaluate return value / status when a concurrent installation is detected #84656

Closed
skh opened this issue Dec 1, 2020 · 7 comments
Closed
Labels
discuss Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@skh
Copy link
Contributor

skh commented Dec 1, 2020

Currently, when a package installation detects that the same package is already being installed, and a certain timeout not yet exceeded, the API call returns early with the list of assets that are currently installed for that package. That list may even be empty, or it may contain all package assets, depending on how far the concurrent package installation had already proceeded. The caller has no way of knowing from the returned list if it comes from a real installation, or from an early return. This was implemented in #84190 .

The reason we decided for this behaviour is that a concurrent package installation is a transient situation that is not an error, and callers should not have to worry about handling this case. Also, not introducing a new error case or different return format kept the changes in #84190 small.

This issue is to discuss if we want to change this behavior and

  • return a different status when the installation was not attempted because a concurrent installation was detected
  • react accordingly in the UI to notify the user that the package installation might have had a problem.
@skh skh added discuss Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Team:Fleet Team label for Observability Data Collection Fleet team labels Dec 1, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@ph
Copy link
Contributor

ph commented Dec 1, 2020

What is the current behavior at the moment? @jfsiii or @neptunian wdyt?

@skh
Copy link
Contributor Author

skh commented Dec 1, 2020

@ph The current behavior is described in the initial description.

@neptunian
Copy link
Contributor

neptunian commented Dec 4, 2020

I think it's unexpected to attempt to install a package and get a response with no way of knowing that the package has not actually finished installing and the asset list is partial. The concern is the caller might act on the knowledge that the package has installed successfully when the package could actually fail since it's still installing. So they might get "package has successfully installed" in the UI which could be untrue. Ideally we could somehow track the current installation process and return the final result (wait for the status to change and give up after some time?). Otherwise I think returning a 409 (or something else) with a description as you described above would be safer.

@skh
Copy link
Contributor Author

skh commented Dec 7, 2020

The concern is the caller might act on the knowledge that the package has installed successfully when the package could actually fail since it's still installing.

That's a really good point, thanks for your input. I'll have a PR up soon.

@skh
Copy link
Contributor Author

skh commented Jan 18, 2021

Closing, the endpoint now returns a 409 error when a concurrent installation is detected.

@skh skh closed this as completed Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

4 participants