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

feat: add command to install appmap agent #467

Merged
merged 18 commits into from
Oct 6, 2022

Conversation

ahtrotta
Copy link
Contributor

Fixes applandinc/board#161

@ahtrotta ahtrotta force-pushed the add-command-to-install-appmap-agent branch from 194a64a to 8829a07 Compare September 22, 2022 13:27
@ahtrotta ahtrotta marked this pull request as ready for review September 22, 2022 20:12
src/extension.ts Outdated
await workspaceServices.enroll(processService);
})();
processService.onReady(activateUptodateService);
await processService.install();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awaiting an install here may take a significant amount of time depending on the users connection/platform. The problem with this is that any initialization logic below this will be blocked until the installation completes. In the previous implementation the installation and enrollment is done without awaiting the result, allowing the rest of the initialization to continue on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that if we didn't block the rest of the initialization logic, then the user might try to press the automated install button before the installAgent command gets created and then it would fail. Do you think that's a problem?

@dustinbyrne dustinbyrne force-pushed the add-command-to-install-appmap-agent branch 2 times, most recently from 2b050dc to b395a6d Compare October 5, 2022 21:53
@dustinbyrne dustinbyrne self-requested a review October 6, 2022 13:29
@dustinbyrne dustinbyrne merged commit b5e0933 into master Oct 6, 2022
@dustinbyrne dustinbyrne deleted the add-command-to-install-appmap-agent branch October 6, 2022 13:29
appland-release pushed a commit that referenced this pull request Oct 6, 2022
# [0.45.0](v0.44.1...v0.45.0) (2022-10-06)

### Bug Fixes

* Fix crash on undefined object ([a0e7c41](a0e7c41))
* Fix multi-line processing of uptodate output ([ba42826](ba42826))

### Features

* Add command to install appmap agent ([#467](#467)) ([b5e0933](b5e0933))
* Authenticate with AppMap Server ([ac971f3](ac971f3))
* manage AppMap Server configuration ([f08b5e6](f08b5e6))
@appland-release
Copy link
Contributor

🎉 This PR is included in version 0.45.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants