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

Add Edge Scripting API #145

Merged
merged 12 commits into from
Nov 19, 2024
Merged

Add Edge Scripting API #145

merged 12 commits into from
Nov 19, 2024

Conversation

ToshY
Copy link
Owner

@ToshY ToshY commented Nov 17, 2024

Fixes #144


TODO:

  • Implement "Edge Scripting API"
  • Update docs
  • Resolve PR comments
  • Manual testing

@ToshY ToshY added the wip Work in progress label Nov 17, 2024
@ToshY ToshY self-assigned this Nov 17, 2024
@ToshY ToshY changed the title setup edge scripting api base class and api docs Add Edge Scripting API Nov 17, 2024
@ToshY
Copy link
Owner Author

ToshY commented Nov 17, 2024

Initially thought I had to create this from scratch, but it seems that what was previously known (or available in the API docs) as "Compute scripts" which has now been transformed to "Edge scripts", which can be seen from the endpoints being the same: /compute/script.

Example:

So the majority of the existing "Compute" models and endpoints should be moved and renamed to "Edge script" standards. This most likely would be classified as a breaking change, as all existing "Compute" would be moved to the "Edge Scripting" namespace, and would require an instance of the EdgeScriptingAPI class instead of the BaseAPI class.

(As "compute scripts" can not be found searching through the API docs, I'm not sure if it is actually used by anyone. Still a breaking change if it's refactored though; 5.0.0).

@ToshY ToshY added this to the 5.0 milestone Nov 17, 2024
@ToshY ToshY force-pushed the feature/144-edge-scripting branch from 0fb015e to 093914f Compare November 17, 2024 14:43
@ToshY ToshY force-pushed the feature/144-edge-scripting branch from fb97421 to 7e618a2 Compare November 17, 2024 18:18
docs/edge-scripting-api.md Outdated Show resolved Hide resolved
docs/edge-scripting-api.md Outdated Show resolved Hide resolved
docs/base-api.md Show resolved Hide resolved
Copy link
Owner Author

@ToshY ToshY left a comment

Choose a reason for hiding this comment

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

lgtm.

Left todo is testing, and likely fix some issues along the way.

Edit

Tested and already looked good at 7b7d80c

Surprisingly, didn't have to do many code changes. Manual testing seems OK.

@ToshY ToshY marked this pull request as ready for review November 18, 2024 21:57
@ToshY ToshY removed the wip Work in progress label Nov 18, 2024
@ToshY
Copy link
Owner Author

ToshY commented Nov 18, 2024

Think merge it tomorrow 💭

Don't forget to update the UPGRADE.md regarding breaking change from compute to edge scripting and introduction of integration endpoint in base.

@ToshY ToshY merged commit 97fe811 into master Nov 19, 2024
15 checks passed
@ToshY ToshY deleted the feature/144-edge-scripting branch November 19, 2024 18:53
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.

[Feature]: Edge Scripting
1 participant