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

Custom Worker - Warning when use of identifiers. #896

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

goetzrrGit
Copy link
Contributor

@goetzrrGit goetzrrGit commented Sep 19, 2023

Closes #897
During expansion logic, I observed that when the user attempts to utilize the activity's parameters as command arguments, the custom worker throws an error. This occurs because it endeavors to assess the value of the parameters, which haven't been defined yet.

Unfortunately, the custom worker lacks the capability to evaluate identifiers without compiling the user code at runtime, which is not a viable option.

To address this, I have implemented a warning in case this situation arises.

ex.
previous

let temp = 100
C.PREHEAT_OVEN(temp) #ERROR

now

C.PREHEAT_OVEN(temp) #warning Not able to evaluate identifiers

@goetzrrGit goetzrrGit requested a review from a team as a code owner September 19, 2023 17:15
@goetzrrGit goetzrrGit temporarily deployed to test-workflow September 19, 2023 17:15 — with GitHub Actions Inactive
@goetzrrGit goetzrrGit added bug Something isn't working fix A bug fix labels Sep 19, 2023
@mattdailis
Copy link
Collaborator

Sounds like a reasonable approach to me. We can't currently statically determine that an activity's parameters will be within the allowed range for the command.

Related question - what happens if an activity argument turns out to be out of range for the given command? Will the service still produce SEQ JSON, or will it abort with an error?

@goetzrrGit goetzrrGit force-pushed the identifier_in_customworker branch from c6fb4a5 to e26ca10 Compare September 19, 2023 20:07
@goetzrrGit goetzrrGit temporarily deployed to test-workflow September 19, 2023 20:07 — with GitHub Actions Inactive
src/workers/customCodes.ts Outdated Show resolved Hide resolved
@goetzrrGit
Copy link
Contributor Author

@mattdailis The service will still produce a SeqJson file. It will just fail downstream at the seqgen level as seqgen will say the values is out of range.

@goetzrrGit goetzrrGit force-pushed the identifier_in_customworker branch from e26ca10 to 4a91f68 Compare September 21, 2023 21:51
@goetzrrGit goetzrrGit temporarily deployed to test-workflow September 21, 2023 21:51 — with GitHub Actions Inactive
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

LGMT if @mattdailis is alright with it as well. Thank you!

…f variables.

ex.

let temp = 100
C.PREHEAT_OVEN(temp) #ERROR

now
C.PREHEAT_OVEN(temp) #warning
@goetzrrGit goetzrrGit force-pushed the identifier_in_customworker branch from 4a91f68 to 8baa494 Compare September 25, 2023 17:04
@goetzrrGit goetzrrGit temporarily deployed to test-workflow September 25, 2023 17:04 — with GitHub Actions Inactive
@goetzrrGit goetzrrGit merged commit d79d586 into develop Sep 25, 2023
4 checks passed
@goetzrrGit goetzrrGit deleted the identifier_in_customworker branch September 25, 2023 20:20
JosephVolosin pushed a commit that referenced this pull request Aug 20, 2024
Fix the bug where the custom worker will throw an error for the use of variables.

ex.

let temp = 100
C.PREHEAT_OVEN(temp) #ERROR

now
C.PREHEAT_OVEN(temp) #warning
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
Fix the bug where the custom worker will throw an error for the use of variables.

ex.

let temp = 100
C.PREHEAT_OVEN(temp) #ERROR

now
C.PREHEAT_OVEN(temp) #warning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix A bug fix
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Custom Worker - Identifiers throw an error
3 participants