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

Distribute modules as binaries #152

Merged
merged 19 commits into from
Nov 17, 2023
Merged

Conversation

DenysGonchar
Copy link
Collaborator

@DenysGonchar DenysGonchar commented Oct 23, 2023

This PR introduces the following changes:

  • Cleanup of the integration testing
  • Since we moved REST API out of this repo, all the leftovers are removed
  • Extraction of amoc_code_server functionality from of amoc_scenario module:
    • now amoc_scenario is just a wrapper for behavior.
    • and amoc_code_server is responsible for modules' analysis and distribution of uploaded modules.
  • Modules' distribution is now based on binary representation, so it's language agnostic and can be reused for elixir.
  • comparison of the modules is based on MD5
  • amoc_code_server_SUITE covers all the basic use cases and demonstrates some of the corner cases that are covered in amoc_code_server

@DenysGonchar DenysGonchar marked this pull request as ready for review October 23, 2023 07:12
Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

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

Couple of small comments, plus two more points here:

  • There's a page with all telemetry events exposed by amoc, all the telemetry changes and additions in this PR should be well documented there, so that writing handlers doesn't require browsing through the code.
  • amoc_scenario:terminate/1 isn't documented nor verified as optional.

Otherwise this PR is looking brilliant! 🤩

src/amoc_coordinator/amoc_coordinator.erl Show resolved Hide resolved
src/amoc_code_server.erl Outdated Show resolved Hide resolved
src/amoc_code_server.erl Outdated Show resolved Hide resolved
@DenysGonchar
Copy link
Collaborator Author

Couple of small comments, plus two more points here:

* There's a page with all telemetry events exposed by amoc, all the telemetry changes and additions in this PR should be well documented there, so that writing handlers doesn't require browsing through the code.

* `amoc_scenario:terminate/1` isn't documented nor verified as optional.

Otherwise this PR is looking brilliant! 🤩

telemetry.md is already updated.

@NelsonVides
Copy link
Collaborator

Couple of small comments, plus two more points here:

* There's a page with all telemetry events exposed by amoc, all the telemetry changes and additions in this PR should be well documented there, so that writing handlers doesn't require browsing through the code.

* `amoc_scenario:terminate/1` isn't documented nor verified as optional.

Otherwise this PR is looking brilliant! 🤩

telemetry.md is already updated.

Ah, see it now. Sorry, disadvantages of such a huge PR, too much diff to browse 🥲

Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

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

Absolutely fantastic, thank you for this huge work! 🤩

@NelsonVides NelsonVides merged commit bc377e7 into master Nov 17, 2023
4 checks passed
@NelsonVides NelsonVides deleted the distribute-modules-as-binaries branch November 17, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants