Skip to content

Conversation

@rjd15372
Copy link
Member

This commit adds script function flags to the module API, which allows function scripts to specify the function flags programmatically.

When the scripting engine compiles the script code can extract the flags from the code and set the flags on the compiled function objects.

This commit adds script function flags to the module API, which allows
function scripts to specify the function flags programmatically.

When the scripting engine compiles the script code can extract the flags
from the code and set the flags on the compiled function objects.

Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372 rjd15372 requested a review from zuiderkwast November 13, 2025 17:17
@rjd15372 rjd15372 self-assigned this Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.42%. Comparing base (01a7657) to head (a85185c).
⚠️ Report is 13 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2836      +/-   ##
============================================
- Coverage     72.46%   72.42%   -0.04%     
============================================
  Files           128      128              
  Lines         70364    70371       +7     
============================================
- Hits          50986    50965      -21     
- Misses        19378    19406      +28     
Files with missing lines Coverage Δ
src/script.c 80.13% <ø> (+0.91%) ⬆️

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Looks fine, but why do scripts need to be able to declare per these flags per function? Do you have a real use case?

@rjd15372
Copy link
Member Author

Looks fine, but why do scripts need to be able to declare per these flags per function? Do you have a real use case?

These are only used for FUNCTION LOAD scripts, where script functions are declared dynamically using the scripting server API. For EVAL scripts, these flags are parsed from the shebang header.

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Nov 18, 2025

These are only used for FUNCTION LOAD scripts, where script functions are declared dynamically using the scripting server API. For EVAL scripts, these flags are parsed from the shebang header.

In the FUNCTION LOAD docs, the shebang is used too:

FUNCTION LOAD "#!lua name=mylib \n server.register_function('myfunc', function(keys, args) return args[1] end)"

What am I missing?

@rjd15372
Copy link
Member Author

What am I missing?

See "Function Flags" section in https://valkey.io/topics/functions-intro/

Example:

server.register_function('my_hset', my_hset)
server.register_function{
  function_name='my_hgetall',
  callback=my_hgetall,
  flags={ 'no-writes' }
}
server.register_function{
  function_name='my_hlastmodified',
  callback=my_hlastmodified,
  flags={ 'no-writes' }
}

@zuiderkwast
Copy link
Contributor

Thank you! Now I understand.

The page has this disclaimer:

This page is under review. The page is likely incorrect, contains invalid links, and/or needs technical review. In the future it may change substantially or be removed entirely.

If you have read this page and found nothing wrong, let's remove the disclaimer?

@rjd15372
Copy link
Member Author

If you have read this page and found nothing wrong, let's remove the disclaimer?

Sure, I can proof read it and then remove the disclaimer.

@rjd15372
Copy link
Member Author

@zuiderkwast is anything still missing or can this PR be approved?

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

is anything still missing or can this PR be approved?

Only that I need to take the time to review it. :)

@rjd15372 rjd15372 merged commit 05540af into valkey-io:unstable Nov 20, 2025
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants