-
Notifications
You must be signed in to change notification settings - Fork 88
feat: Knative deployment support #206
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
base: master
Are you sure you want to change the base?
Conversation
496b8c1 to
fca8fa2
Compare
68b7d33 to
ac9a9ab
Compare
Signed-off-by: Abhishek Kumar <[email protected]>
7030290 to
5f13fc7
Compare
Signed-off-by: Abhishek Kumar <[email protected]>
|
there are two things I want to discuss with you on wrappers and code packaging, Since the |
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe update introduces comprehensive support for Knative within the SeBS platform, featuring new cloud event handlers, MinIO storage interactions, deployment configurations, and extensive testing capabilities. The changes span Node.js and Python environments, providing robust functionality for event handling, storage, and deployment automation. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 28
Outside diff range and nitpick comments (22)
sebs/knative/storage.py (3)
8-8: Consider adding a class docstring.Adding a docstring to the
KnativeMinioclass can provide better context and understanding of its purpose and usage.""" KnativeMinio extends the Minio class to provide Knative-specific storage functionalities. """
13-19: Consider adding a docstring for the__init__method.Adding a docstring for the
__init__method can help clarify the purpose of each parameter.""" Initialize KnativeMinio. Parameters: - docker_client: Docker client instance. - cache_client: Cache client instance. - res: Resources instance. - replace_existing: Boolean flag to indicate if existing storage should be replaced. """
22-28: Consider adding a docstring for thedeserializemethod.Adding a docstring for the
deserializemethod can help clarify its functionality.""" Deserialize the KnativeMinio instance from cached configuration. Parameters: - cached_config: Cached Minio configuration. - cache_client: Cache client instance. - resources: Resources instance. Returns: - KnativeMinio instance. """benchmarks/wrappers/knative/python/func.py (1)
8-42: Consider refining error handling and logging.To provide more detailed error information and improve logging, you can include the stack trace in the log and return a more descriptive error message.
- logging.error(f"Error - invocation failed! Reason: {e}") + logging.exception("Error - invocation failed!") - response = { - "begin": begin.strftime("%s.%f"), - "end": end.strftime("%s.%f"), - "results_time": results_time, - "result": f"Error - invocation failed! Reason: {e}", - } + response = jsonify({ + "begin": begin.strftime("%s.%f"), + "end": end.strftime("%s.%f"), + "results_time": results_time, + "error": str(e), + "stack_trace": traceback.format_exc(), + })sebs/knative/function.py (9)
11-33: UseX | Yfor type annotations.To follow modern Python type annotation practices, use
X | Yinstead ofOptional[X].- storage: Optional[MinioConfig] = None + storage: MinioConfig | None = NoneTools
Ruff
15-15: Use
X | Yfor type annotationsConvert to
X | Y(UP007)
18-25: Consider adding a docstring for thedeserializemethod.Adding a docstring for the
deserializemethod can help clarify its functionality.""" Deserialize the KnativeFunctionConfig instance from a dictionary. Parameters: - data: Dictionary containing the configuration data. Returns: - KnativeFunctionConfig instance. """
26-27: Consider adding a docstring for theserializemethod.Adding a docstring for the
serializemethod can help clarify its functionality.""" Serialize the KnativeFunctionConfig instance to a dictionary. Returns: - Dictionary containing the serialized data. """
29-33: Consider adding a docstring for thefrom_benchmarkmethod.Adding a docstring for the
from_benchmarkmethod can help clarify its functionality.""" Create a KnativeFunctionConfig instance from a benchmark. Parameters: - benchmark: Benchmark instance. Returns: - KnativeFunctionConfig instance. """
37-43: Consider adding a docstring for the__init__method.Adding a docstring for the
__init__method can help clarify the purpose of each parameter.""" Initialize KnativeFunction. Parameters: - name: Name of the function. - benchmark: Benchmark associated with the function. - code_package_hash: Hash of the code package. - cfg: KnativeFunctionConfig instance. """
47-48: Consider adding a docstring for theconfigproperty.Adding a docstring for the
configproperty can help clarify its functionality.""" Get the KnativeFunctionConfig instance associated with this function. Returns: - KnativeFunctionConfig instance. """
50-52: Consider adding a docstring for thetypenamemethod.Adding a docstring for the
typenamemethod can help clarify its functionality.""" Get the type name of this function. Returns: - String representing the type name. """
54-55: Consider adding a docstring for theserializemethod.Adding a docstring for the
serializemethod can help clarify its functionality.""" Serialize the KnativeFunction instance to a dictionary. Returns: - Dictionary containing the serialized data. """
57-78: Consider adding a docstring for thedeserializemethod.Adding a docstring for the
deserializemethod can help clarify its functionality.""" Deserialize the KnativeFunction instance from a dictionary. Parameters: - cached_config: Dictionary containing the cached configuration data. Returns: - KnativeFunction instance. """tools/knative_preparation.py (6)
4-9: Consider improving error handling and logging.To provide more detailed error information and improve logging, you can include the stack trace in the log and return a more descriptive error message.
- print(f"Error occurred: {e}") + print(f"Error occurred: {e}", file=sys.stderr) + traceback.print_exc()
14-20: Consider refining the logging messages.To provide more detailed information, you can include the command being executed in the log messages.
- print("Installing Minikube...") + print("Installing Minikube using curl...")
22-30: Consider refining the logging messages.To provide more detailed information, you can include the command being executed in the log messages.
- print("Installing kubectl...") + print("Installing kubectl using curl...")
32-39: Consider refining the logging messages.To provide more detailed information, you can include the command being executed in the log messages.
- print("Installing Cosign...") + print("Installing Cosign using curl...")
41-46: Consider refining the logging messages.To provide more detailed information, you can include the command being executed in the log messages.
- print("Installing jq...") + print("Installing jq using apt-get...")
48-73: Consider refining the logging messages.To provide more detailed information, you can include the command being executed in the log messages.
- print("Extracting images from the manifest and verifying signatures...") + print("Extracting images from the manifest and verifying signatures using curl...") - print("Installing Knative Serving component...") + print("Installing Knative Serving component using kubectl...") - print("Installing Knative Kourier networking layer...") + print("Installing Knative Kourier networking layer using kubectl...") - print("Fetching External IP address or CNAME...") + print("Fetching External IP address or CNAME using kubectl...") - print("Verifying Knative Serving installation...") + print("Verifying Knative Serving installation using kubectl...")sebs/knative/triggers.py (3)
31-31: Clarify function name.The function name
get_commandis confusing as it returns a list of arguments, not a command.Consider renaming it to
get_argumentsfor better clarity.
54-54: Use f-string instead offormatcall.Convert the
formatcall to an f-string for better readability.- self.logging.error("Invocation of {} failed!".format(self.fname)) + self.logging.error(f"Invocation of {self.fname} failed!")Tools
Ruff
54-54: Use f-string instead of
formatcallConvert to f-string
(UP032)
80-84: Add type hints for constructor parameters.Adding type hints improves code readability and helps with static analysis.
- def __init__(self, fname: str, url: str): + def __init__(self, fname: str, url: str) -> None:
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- benchmarks/wrappers/knative/python/func.py (1 hunks)
- config/systems.json (1 hunks)
- sebs.py (1 hunks)
- sebs/faas/config.py (1 hunks)
- sebs/knative/config.py (1 hunks)
- sebs/knative/function.py (1 hunks)
- sebs/knative/knative.py (1 hunks)
- sebs/knative/storage.py (1 hunks)
- sebs/knative/triggers.py (1 hunks)
- sebs/types.py (1 hunks)
- tools/knative_preparation.py (1 hunks)
Files skipped from review due to trivial changes (1)
- sebs/types.py
Additional context used
Ruff
sebs/knative/function.py
15-15: Use
X | Yfor type annotationsConvert to
X | Y(UP007)
sebs/knative/triggers.py
5-5:
typing.Dictimported but unusedRemove unused import:
typing.Dict(F401)
40-45: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
54-54: Use f-string instead of
formatcallConvert to f-string
(UP032)
sebs/faas/config.py
213-213: Use
config.get(name, config)instead of anifblockReplace with
config.get(name, config)(SIM401)
sebs/knative/config.py
2-2:
sebs.faas.config.Credentialsimported but unusedRemove unused import:
sebs.faas.config.Credentials(F401)
sebs/knative/knative.py
1-1:
codeimported but unusedRemove unused import:
code(F401)
2-2:
os.devnullimported but unusedRemove unused import:
os.devnull(F401)
4-4:
flask.configimported but unusedRemove unused import:
flask.config(F401)
6-6:
sebs.faas.function.ExecutionResultimported but unusedRemove unused import:
sebs.faas.function.ExecutionResult(F401)
13-13:
sebs.knative.triggers.KnativeLibraryTriggerimported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger(F401)
14-14:
sebs.faas.config.Resourcesimported but unusedRemove unused import:
sebs.faas.config.Resources(F401)
15-15:
typing.Dictimported but unusedRemove unused import
(F401)
15-15:
typing.Optionalimported but unusedRemove unused import
(F401)
18-18:
uuidimported but unusedRemove unused import:
uuid(F401)
30-30: Redefinition of unused
configfrom line 4(F811)
53-53: Redefinition of unused
configfrom line 4(F811)
149-154: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
158-158: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
222-222: Undefined name
function_cfg(F821)
232-237: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
240-245: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
255-260: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
286-286: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
292-292: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
305-319: Prefer
capture_outputover sendingstdoutandstderrtoPIPEReplace with
capture_outputkeyword argument(UP022)
326-326: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
333-333: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
352-352: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (9)
tools/knative_preparation.py (1)
11-12: LGTM!The function
is_installedis well-structured and there are no issues.sebs/knative/knative.py (7)
52-55: LGTM!The
configmethod correctly returns the Knative configuration.Tools
Ruff
53-53: Redefinition of unused
configfrom line 4(F811)
57-59: LGTM!The
get_knative_func_cmdmethod correctly returns the Knative function command.
61-64: LGTM!The
function_typemethod correctly returns the specific function type for Knative.
71-86: LGTM!The
find_imagemethod correctly checks if a Docker image exists in the repository.
166-200: Ensure the image tag includesknative.The image tag should include
knativeto avoid mixing images between OpenWhisk and Knative.Ensure that the image tag includes
knative.
364-365: LGTM!The
namemethod correctly returns the name of the system.
27-37: Avoid redefinition of theconfigparameter.The
configparameter is redefined, which can cause confusion and potential issues.- from flask import configLikely invalid or redundant comment.
Tools
Ruff
30-30: Redefinition of unused
configfrom line 4(F811)
sebs.py (1)
91-91: LGTM!The
common_params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- benchmarks/wrappers/knative/python/storage.py (1 hunks)
Additional context used
Ruff
benchmarks/wrappers/knative/python/storage.py
3-3:
jsonimported but unusedRemove unused import:
json(F401)
Additional comments not posted (7)
benchmarks/wrappers/knative/python/storage.py (7)
42-48: LGTM!The
unique_namemethod correctly generates a unique name for a file.
51-54: LGTM!The
uploadmethod correctly handles file uploads and returns the unique key name.
56-57: LGTM!The
downloadmethod correctly handles file downloads.
59-63: LGTM!The
download_directorymethod correctly handles directory downloads.
65-70: LGTM!The
upload_streammethod correctly handles stream uploads and returns the unique key name.
72-74: LGTM!The
download_streammethod correctly handles stream downloads.
76-80: LGTM!The
get_instancemethod correctly implements the singleton pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two general comments:
- Please ensure the docs are fully updated to describe everything relevant about this (pretty extensive) change.
- Some comments from coderabbitai make sense to fix, e.g. unused imports and the like. Have a look through them.
I am left with one file to review - I'll get to it in a few hours' time.
Signed-off-by: Abhishek Kumar <[email protected]>
4f1271e to
8779f50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
Outside diff range and nitpick comments (21)
benchmarks/wrappers/knative/python/func.py (2)
11-29: Suggestion: Move the timing code closer to thehandlercall for accuracy.To ensure accurate timing, move the
beginandendvariables closer to thehandlercall.- begin = datetime.datetime.now() - ret = handler(event) - end = datetime.datetime.now() + ret = handler(event) + end = datetime.datetime.now() + begin = datetime.datetime.now()
31-41: Suggestion: Move the timing code closer to thehandlercall for accuracy.To ensure accurate timing, move the
beginandendvariables closer to thehandlercall.- end = datetime.datetime.now() - results_time = (end - begin) / datetime.timedelta(microseconds=1) + results_time = (end - begin) / datetime.timedelta(microseconds=1) + end = datetime.datetime.now() + begin = datetime.datetime.now()benchmarks/wrappers/knative/nodejs/index.js (2)
7-32: Suggestion: Move the timing code closer to thehandlercall for accuracy.To ensure accurate timing, move the
startTimeandendTimevariables closer to thehandlercall.- const startTime = new Date(); - const result = await handler(eventData); - const endTime = new Date(); + const result = await handler(eventData); + const endTime = new Date(); + const startTime = new Date();
33-50: Suggestion: Move the timing code closer to thehandlercall for accuracy.To ensure accurate timing, move the
startTimeandendTimevariables closer to thehandlercall.- const endTime = new Date(); - const resultTime = (endTime - startTime) / 1000; // Time in seconds + const resultTime = (endTime - startTime) / 1000; // Time in seconds + const endTime = new Date(); + const startTime = new Date();sebs/knative/function.py (3)
11-11: Usefrozen=Truefor immutability.Consider adding
frozen=Trueto the@dataclassdecorator to make instances of this class immutable.- @dataclass + @dataclass(frozen=True)
30-33: Remove unnecessarysupercall.The
supercall is unnecessary in this context and can be simplified.- return super(KnativeFunctionConfig, KnativeFunctionConfig)._from_benchmark( - benchmark, KnativeFunctionConfig - ) + return KnativeFunctionConfig._from_benchmark(benchmark, KnativeFunctionConfig)
37-44: Consider using type hints for parameters.Adding type hints improves readability and helps with static analysis.
- def __init__( - self, - name: str, - benchmark: str, - code_package_hash: str, - cfg: KnativeFunctionConfig, - ): + def __init__(self, name: str, benchmark: str, code_package_hash: str, cfg: KnativeFunctionConfig):tools/knative_preparation.py (1)
8-9: Uselogginginstead ofUsing the
loggingmodule provides more flexibility and control over how messages are logged.- print(f"Error occurred: {e}") + logging.error(f"Error occurred: {e}")sebs/knative/triggers.py (1)
10-14: Consider using type hints for parameters.Adding type hints improves readability and helps with static analysis.
- def __init__(self, fname: str, func_cmd: Optional[List[str]] = None </blockquote></details> <details> <summary>config/systems.json (1)</summary><blockquote> `237-277`: **Ensure completeness of Node.js deployment files.** The `files` array for Node.js deployment is incomplete and contains empty strings. ```diff - "files": [ - "", - "" - ] + "files": [ + "index.js", + "storage.js" + ]Ensure that the correct filenames are added.
sebs/knative/config.py (3)
1-5: Remove unused import.The
MinioConfigimport is unused and should be removed.- from sebs.storage.config import MinioConfig
53-53: Remove unused local variable.The
cached_configvariable is assigned but never used.- cached_config = cache.get_config("knative")
191-191: Remove unused local variable.The
cached_configvariable is assigned but never used.- cached_config = cache.get_config("knative")Tools
Ruff
191-191: Local variable
cached_configis assigned to but never usedRemove assignment to unused variable
cached_config(F841)
sebs/knative/knative.py (8)
13-13: Remove unused import.The
KnativeLibraryTriggerclass is imported but not used anywhere in the file.- from sebs.knative.triggers import KnativeLibraryTrigger, KnativeHTTPTrigger + from sebs.knative.triggers import KnativeHTTPTriggerTools
Ruff
13-13:
sebs.knative.triggers.KnativeLibraryTriggerimported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger(F401)
14-14: Remove unused importstyping.Dictandtyping.Optional.The
DictandOptionaltypes are imported but not used anywhere in the file.- from typing import Dict, Tuple, Type, List, Optional + from typing import Tuple, Type, ListTools
Ruff
14-14:
typing.Dictimported but unusedRemove unused import
(F401)
14-14:
typing.Optionalimported but unusedRemove unused import
(F401)
18-18: Remove redefined import.The
KnativeMinioimport is redefined and should be removed.- from sebs.knative.storage import KnativeMinioTools
Ruff
18-18: Redefinition of unused
KnativeMiniofrom line 12Remove definition:
KnativeMinio(F811)
206-206: Remove unused local variable.The
package_configvariable is assigned but never used.- package_config = CONFIG_FILES[language_name]Tools
Ruff
206-206: Local variable
package_configis assigned to but never usedRemove assignment to unused variable
package_config(F841)
263-263: Remove unused local variable.The
languagevariable is assigned but never used.- language = code_package.language_nameTools
Ruff
263-263: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
316-316: Raise exceptions withraise ... from errorraise ... from None.Exceptions should be raised with
raise ... from errorraise ... from Noneto distinguish them from errors in exception handling.- raise RuntimeError("Failed to access func binary") + raise RuntimeError("Failed to access func binary") from NoneTools
Ruff
316-316: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
356-356: Raise exceptions withraise ... from errorraise ... from None.Exceptions should be raised with
raise ... from errorraise ... from Noneto distinguish them from errors in exception handling.- raise RuntimeError(e) + raise RuntimeError(e) from eTools
Ruff
356-356: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
374-374: Raise exceptions withraise ... from errorraise ... from None.Exceptions should be raised with
raise ... from errorraise ... from Noneto distinguish them from errors in exception handling.- raise RuntimeError(e) + raise RuntimeError(e) from eTools
Ruff
374-374: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- benchmarks/wrappers/knative/nodejs/index.js (1 hunks)
- benchmarks/wrappers/knative/nodejs/storage.js (1 hunks)
- benchmarks/wrappers/knative/python/func.py (1 hunks)
- benchmarks/wrappers/knative/python/storage.py (1 hunks)
- config/systems.json (1 hunks)
- sebs/knative/init.py (1 hunks)
- sebs/knative/config.py (1 hunks)
- sebs/knative/function.py (1 hunks)
- sebs/knative/knative.py (1 hunks)
- sebs/knative/storage.py (1 hunks)
- sebs/knative/triggers.py (1 hunks)
- tools/knative_preparation.py (1 hunks)
Files skipped from review due to trivial changes (1)
- sebs/knative/init.py
Additional context used
Biome
benchmarks/wrappers/knative/nodejs/storage.js
[error] 55-55: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 56-56: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 58-58: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
Ruff
benchmarks/wrappers/knative/python/storage.py
3-3:
jsonimported but unusedRemove unused import:
json(F401)
sebs/knative/config.py
191-191: Local variable
cached_configis assigned to but never usedRemove assignment to unused variable
cached_config(F841)
sebs/knative/knative.py
13-13:
sebs.knative.triggers.KnativeLibraryTriggerimported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger(F401)
14-14:
typing.Dictimported but unusedRemove unused import
(F401)
14-14:
typing.Optionalimported but unusedRemove unused import
(F401)
18-18: Redefinition of unused
KnativeMiniofrom line 12Remove definition:
KnativeMinio(F811)
206-206: Local variable
package_configis assigned to but never usedRemove assignment to unused variable
package_config(F841)
256-256: Undefined name
function_cfg(F821)
263-263: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
316-316: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
356-356: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
374-374: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (19)
sebs/knative/storage.py (3)
10-11: LGTM!The
deployment_namemethod is simple and correct.
13-20: LGTM!The
__init__method correctly initializes theKnativeMinioclass.
23-28: LGTM!The
deserializemethod is correctly implemented and follows the pattern of calling the parent class's method.benchmarks/wrappers/knative/python/func.py (3)
1-5: LGTM!The import statements are correct and necessary for the function implementation.
9-9: LGTM!Setting the logging level at the beginning of the function is a good practice.
29-29: LGTM!The return statements are correctly implemented.
Also applies to: 41-41
benchmarks/wrappers/knative/nodejs/index.js (3)
1-2: LGTM!The import statements are correct and necessary for the function implementation.
10-10: LGTM!Setting up logging configuration within the function is a good practice.
Also applies to: 16-16, 37-37
28-32: LGTM!The return statements are correctly implemented.
Also applies to: 46-50
benchmarks/wrappers/knative/nodejs/storage.js (7)
11-25: LGTM!The constructor is correctly implemented.
27-31: LGTM!The
unique_namemethod is correctly implemented.
33-36: LGTM!The
uploadmethod is correctly implemented.
38-40: LGTM!The
downloadmethod is correctly implemented.
42-47: LGTM!The
uploadStreammethod is correctly implemented.
49-51: LGTM!The
downloadStreammethod is correctly implemented.
63-63: LGTM!The export statement is correctly implemented.
benchmarks/wrappers/knative/python/storage.py (2)
3-3: Remove unused import.The
jsonmodule is imported but not used in the file.- import jsonTools
Ruff
3-3:
jsonimported but unusedRemove unused import:
json(F401)
38-39: Improve exception handling.The current exception handling logs the exception and re-raises it. Consider logging the exception as an error instead of info for better visibility.
- logging.info(e) + logging.error(e)tools/knative_preparation.py (1)
11-12: LGTM!The function
is_installedis straightforward and does not require changes.
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sebs/knative/knative.py (1 hunks)
Additional context used
Ruff
sebs/knative/knative.py
13-13:
sebs.knative.triggers.KnativeLibraryTriggerimported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger(F401)
14-14:
typing.Dictimported but unusedRemove unused import
(F401)
14-14:
typing.Optionalimported but unusedRemove unused import
(F401)
18-18: Redefinition of unused
KnativeMiniofrom line 12Remove definition:
KnativeMinio(F811)
206-206: Local variable
package_configis assigned to but never usedRemove assignment to unused variable
package_config(F841)
256-256: Undefined name
function_cfg(F821)
263-263: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
316-316: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
356-356: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
374-374: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (6)
sebs/knative/knative.py (6)
56-58: LGTM!The method constructs a command list for Knative functions correctly.
65-80: LGTM!The method retrieves or initializes storage for Knative correctly.
82-97: LGTM!The method checks if a Docker image exists in the repository correctly.
385-387: LGTM!The method reinitializes logging handlers for all triggers correctly.
358-381: Usecapture_outputinstead of sendingstdoutandstderrtoPIPE.The
capture_outputargument should be used for better readability and maintainability.- response = subprocess.run( - ["func", "describe", function.name, "--output", "url"], - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, - check=True, - ) + response = subprocess.run( + ["func", "describe", function.name, "--output", "url"], + capture_output=True, + check=True, + )Likely invalid or redundant comment.
Tools
Ruff
374-374: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
318-357: Usecapture_outputinstead of sendingstdoutandstderrtoPIPE.The
capture_outputargument should be used for better readability and maintainability.- subprocess.run( - [ - "func", - "deploy", - "--path", - code_package.code_location, - "--image", - docker_image, - "--name", - function.name, - ], - stderr=subprocess.PIPE, - stdout=subprocess.PIPE, - check=True, - ) + subprocess.run( + [ + "func", + "deploy", + "--path", + code_package.code_location, + "--image", + docker_image, + "--name", + function.name, + ], + capture_output=True, + check=True, + )Likely invalid or redundant comment.
Tools
Ruff
356-356: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tools/knative_preparation.py (1 hunks)
Additional comments not posted (5)
tools/knative_preparation.py (5)
15-16: LGTM!The function is simple and correctly checks if a command is available in the system's PATH.
18-24: LGTM! Ensurerun_commandhas error handling.The function correctly installs Minikube if it is not already installed. Ensure
run_commandhas proper error handling as suggested.
26-34: LGTM! Ensurerun_commandhas error handling.The function correctly installs kubectl if it is not already installed. Ensure
run_commandhas proper error handling as suggested.
36-43: LGTM! Ensurerun_commandhas error handling.The function correctly installs Cosign if it is not already installed. Ensure
run_commandhas proper error handling as suggested.
72-79: LGTM!The main execution block correctly calls the installation functions in sequence.
Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- benchmarks/000.microbenchmarks/010.sleep/input.py (1 hunks)
- benchmarks/000.microbenchmarks/020.network-benchmark/input.py (1 hunks)
- benchmarks/000.microbenchmarks/030.clock-synchronization/input.py (1 hunks)
- benchmarks/000.microbenchmarks/040.server-reply/input.py (1 hunks)
- benchmarks/wrappers/knative/python/func.py (1 hunks)
- config/example.json (1 hunks)
- config/systems.json (1 hunks)
- install.py (3 hunks)
- requirements.knative.txt (1 hunks)
- sebs/config.py (2 hunks)
- sebs/knative/init.py (1 hunks)
- sebs/knative/config.py (1 hunks)
- sebs/knative/function.py (1 hunks)
- sebs/knative/knative.py (1 hunks)
- sebs/knative/triggers.py (1 hunks)
- sebs/sebs.py (1 hunks)
- sebs/utils.py (2 hunks)
- tools/knative_preparation.py (1 hunks)
Files skipped from review due to trivial changes (3)
- requirements.knative.txt
- sebs/config.py
- sebs/knative/init.py
Files skipped from review as they are similar to previous changes (4)
- benchmarks/wrappers/knative/python/func.py
- sebs/knative/function.py
- sebs/knative/triggers.py
- tools/knative_preparation.py
Additional context used
Ruff
sebs/knative/config.py
213-213: Local variable
cached_configis assigned to but never usedRemove assignment to unused variable
cached_config(F841)
sebs/knative/knative.py
49-49: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
213-213: Local variable
CONFIG_FILESis assigned to but never usedRemove assignment to unused variable
CONFIG_FILES(F841)
294-294: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
359-359: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
395-395: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
399-399: Local variable
docker_imageis assigned to but never usedRemove assignment to unused variable
docker_image(F841)
419-419: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
424-424: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
443-443: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (20)
benchmarks/000.microbenchmarks/020.network-benchmark/input.py (1)
6-6: Ensure that thebenchmarks_bucketparameter is utilized if needed.The parameter
benchmarks_bucketis added but not used within the function. Verify if this parameter is necessary or if it should be used within the function logic.benchmarks/000.microbenchmarks/030.clock-synchronization/input.py (1)
6-6: Ensure that thebenchmarks_bucketparameter is utilized if needed.The parameter
benchmarks_bucketis added but not used within the function. Verify if this parameter is necessary or if it should be used within the function logic.benchmarks/000.microbenchmarks/010.sleep/input.py (1)
11-11: Ensure that thebenchmarks_bucketparameter is utilized if needed.The parameter
benchmarks_bucketis added but not used within the function. Verify if this parameter is necessary or if it should be used within the function logic.benchmarks/000.microbenchmarks/040.server-reply/input.py (1)
11-11: Ensure that thebenchmarks_bucketparameter is utilized if needed.The parameter
benchmarks_bucketis added but not used within the function. Verify if this parameter is necessary or if it should be used within the function logic.config/example.json (1)
91-109: Ensure correctness of Knative deployment configuration.The Knative deployment configuration includes properties such as
shutdownStorage,removeCluster,knativeExec,docker_registry, andstorage. Ensure that these configurations are correctly implemented and consistent with other deployments.Verification successful
Knative deployment configuration is correct and complete.
The Knative deployment configuration includes all the necessary properties (
shutdownStorage,removeCluster,knativeExec,docker_registry, andstorage) and their values are correctly set.
config/example.json: Lines 91-109Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of Knative deployment configuration. # Test: Check if the Knative configuration includes all the necessary properties and values. rg --json --pretty 'knative' config/example.jsonLength of output: 1064
Script:
#!/bin/bash # Description: Verify the presence and correctness of all properties in the Knative deployment configuration. # Check for the presence and values of all properties within the "knative" configuration. rg '"knative": {' -A 20 config/example.jsonLength of output: 133
Script:
#!/bin/bash # Description: Verify the presence and correctness of all properties in the Knative deployment configuration. # Check for the presence and values of all properties within the "knative" configuration. rg '"knative": \{' -A 25 config/example.jsonLength of output: 550
install.py (1)
66-71: Ensure correctness of Knative dependencies installation.The new lines handle the installation of Knative-specific dependencies. Ensure that the installation commands are correct and consistent with the existing logic for other deployments.
config/systems.json (1)
237-272: Ensure completeness and correctness of Knative configuration.The newly added lines handle the configuration for Knative, including base images, deployment files, and packages. Ensure that the configurations are complete, correct, and consistent with other systems.
Verification successful
Ensure completeness and correctness of Knative configuration.
The Knative configuration in
config/systems.jsonincludes the necessary properties and values forpythonandnodejslanguages. It is consistent with the configurations of other systems like AWS, Azure, GCP, and OpenWhisk. Each configuration block includesbase_images,images,username, anddeployment(withfilesandpackages), which aligns with the patterns observed in other systems.
Knative Configuration:
- Python: Includes
base_images,images,username, anddeployment(withfilesandpackages).- Nodejs: Includes
base_images,images,username, anddeployment(withfilesandpackages).Comparison with Other Systems:
- All systems have similar structure and properties, ensuring consistency.
No issues were found with the Knative configuration. It appears complete and consistent with other system configurations.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the completeness and correctness of Knative configuration. # Test: Check if the Knative configuration includes all the necessary properties and values. rg --json --pretty 'knative' config/systems.jsonLength of output: 840
Script:
#!/bin/bash # Extract the entire Knative configuration block ast-grep --lang json --pattern '{ "knative": $_ }' config/systems.json # Extract configuration blocks of other systems for comparison ast-grep --lang json --pattern '{ $_: { "languages": $_ } }' config/systems.jsonLength of output: 623
Script:
#!/bin/bash # Extract the entire Knative configuration block ast-grep --lang json --pattern '{ "knative": { "languages": $_ } }' config/systems.json # Extract configuration blocks of other systems for comparison ast-grep --lang json --pattern '{ "languages": $_ }' config/systems.jsonLength of output: 174
Script:
#!/bin/bash # Extract and display context around the knative configuration for manual inspection rg -A 50 'knative' config/systems.jsonLength of output: 787
Script:
#!/bin/bash # Extract and display configurations of other systems for comparison rg -A 50 '"languages"' config/systems.jsonLength of output: 5596
sebs/sebs.py (1)
107-112: Ensure correctness of Knative integration.The newly added lines handle the integration of Knative as a deployment option. Ensure that the integration logic is correct and consistent with the existing logic for other deployments.
sebs/utils.py (1)
92-99: LGTM!The function
sanitize_benchmark_name_for_knativecorrectly sanitizes benchmark names for Knative.sebs/knative/config.py (3)
9-27: LGTM!The class
KnativeCredentialscorrectly handles Docker credentials for Knative.
30-173: LGTM!The class
KnativeResourcescorrectly handles Docker registry and Minio storage configurations for Knative.
176-227: LGTM!The class
KnativeConfigcorrectly handles the overall configuration for Knative, including resources and credentials.Tools
Ruff
213-213: Local variable
cached_configis assigned to but never usedRemove assignment to unused variable
cached_config(F841)
sebs/knative/knative.py (8)
24-48: LGTM!The
__init__method correctly initializes theKnativeobject and handles Docker login if credentials are provided.
49-51: LGTM!The
initializemethod correctly callsinitialize_resourceswith an optional resource prefix.Tools
Ruff
49-49: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
56-71: LGTM!The
get_storagemethod correctly retrieves or initializes Knative Minio storage based on the provided configuration.
73-80: LGTM!The
shutdownmethod correctly stops storage if required and deletes the Knative cluster if configured.
115-189: LGTM!The
build_base_imagemethod correctly builds the base image for the function using the 'func build' command.
454-457: LGTM!The
cached_functionmethod correctly sets logging handlers for HTTP triggers of a function.
458-462: LGTM!The
default_function_namemethod correctly generates a default function name based on the benchmark and language details.
361-396: Usecapture_outputinstead of sendingstdoutandstderrtoPIPE.The
capture_outputargument should be used for better readability and maintainability.- subprocess.run( - [ - *self.get_knative_func_cmd(), - "deploy", - "--path", - code_package.code_location, - ], - stderr=subprocess.PIPE, - stdout=subprocess.PIPE, - check=True, - ) + subprocess.run( + [ + *self.get_knative_func_cmd(), + "deploy", + "--path", + code_package.code_location, + ], + capture_output=True, + check=True, + )Likely invalid or redundant comment.
Tools
Ruff
395-395: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 42
Outside diff range, codebase verification and nitpick comments (4)
sebs/knative/config.py (1)
119-135: Improve logging for storage configuration updates.Enhance the logging messages to provide more context about the storage configuration updates.
- ret.logging.info( - "User-provided configuration is different from cached storage, " - "we will update existing Knative function." - ) + ret.logging.info( + "User-provided storage configuration differs from cached storage. " + "Updating existing Knative function with new storage configuration." + )docs/platforms.md (3)
339-345: Use consistent list style for prerequisites.The list style for prerequisites should be consistent with the rest of the document. Replace dashes with asterisks.
- - `kubectl` - - `kind` - - `helm` - - `jq` - - `func` (It's a [Knative function CLI](https://knative.dev/docs/functions/install-func/#installing-the-func-cli) that is used to interact with your Knative functions directly from your command line. SeBS also makes use of this tool to `build` and `deploy` the benchmarks as Knative functions.) - - `minio` + * `kubectl` + * `kind` + * `helm` + * `jq` + * `func` (It's a [Knative function CLI](https://knative.dev/docs/functions/install-func/#installing-the-func-cli) that is used to interact with your Knative functions directly from your command line. SeBS also makes use of this tool to `build` and `deploy` the benchmarks as Knative functions.) + * `minio`Tools
Markdownlint
340-340: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
341-341: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
342-342: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
343-343: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
344-344: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
345-345: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
349-355: Clarify the path tokubeconfig.yaml.The instruction to export the
KUBECONFIGpath should specify the exact location of thebinfolder.- export KUBECONFIG=path/to/bin/folder/kubeconfig.yaml + export KUBECONFIG=$(pwd)/bin/kubeconfig.yaml
386-387: Clarify the benchmark deployment example.The example command should include a brief explanation of each argument.
- ./sebs.py benchmark invoke 311.compression test --config config/example.json --deployment knative --language python --language-version 3.9 + # Example command to deploy a benchmark to the Knative-enabled cluster + ./sebs.py benchmark invoke 311.compression test \ + --config config/example.json \ + --deployment knative \ + --language python \ + --language-version 3.9
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- benchmarks/wrappers/knative/nodejs/index.js (1 hunks)
- benchmarks/wrappers/knative/nodejs/storage.js (1 hunks)
- benchmarks/wrappers/knative/python/func.py (1 hunks)
- benchmarks/wrappers/knative/python/storage.py (1 hunks)
- config/systems.json (1 hunks)
- docs/platforms.md (1 hunks)
- sebs/benchmark.py (1 hunks)
- sebs/knative/config.py (1 hunks)
- sebs/knative/function.py (1 hunks)
- sebs/knative/knative.py (1 hunks)
- sebs/knative/triggers.py (1 hunks)
- sebs/regression.py (2 hunks)
- tools/knative_setup.sh (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- config/systems.json
- sebs/knative/function.py
Additional context used
Biome
benchmarks/wrappers/knative/nodejs/storage.js
[error] 52-52: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 53-53: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 55-55: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
Ruff
benchmarks/wrappers/knative/python/func.py
5-5:
flask.jsonifyimported but unusedRemove unused import:
flask.jsonify(F401)
7-7:
minioimported but unusedRemove unused import:
minio(F401)
benchmarks/wrappers/knative/python/storage.py
3-3:
jsonimported but unusedRemove unused import:
json(F401)
sebs/knative/config.py
215-215: Local variable
cached_configis assigned to but never usedRemove assignment to unused variable
cached_config(F841)
sebs/knative/knative.py
52-52: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
348-348: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
424-424: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
460-460: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
466-466: Local variable
docker_imageis assigned to but never usedRemove assignment to unused variable
docker_image(F841)
488-488: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
495-495: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
519-519: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Shellcheck
tools/knative_setup.sh
[warning] 28-28: HELM appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 29-29: JQ appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 48-48: red appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 54-54: grey appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 56-56: yellow appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 71-71: Declare and assign separately to avoid masking return values.
(SC2155)
Markdownlint
docs/platforms.md
340-340: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
341-341: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
342-342: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
343-343: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
344-344: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
345-345: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
Additional comments not posted (27)
benchmarks/wrappers/knative/python/storage.py (1)
48-51: Add error handling for upload method.Consider adding error handling for the
uploadmethod to handle potential exceptions from the Minio client.try: self.client.fput_object(bucket, key_name, filepath) except Exception as e: logging.error(f"Failed to upload file {file} to bucket {bucket}: {e}") raise eLikely invalid or redundant comment.
sebs/knative/config.py (3)
9-13: Ensure sensitive data is handled securely.The
KnativeCredentialsclass stores sensitive data likedocker_usernameanddocker_password. Ensure these are handled securely.
[security]
32-43: Ensure sensitive data is handled securely.The
KnativeResourcesclass stores sensitive data likedocker_usernameanddocker_password. Ensure these are handled securely.
[security]
182-189: Ensure sensitive data is handled securely.The
KnativeConfigclass stores sensitive data likeknativeExec,shutdownStorage, andremoveCluster. Ensure these are handled securely.
[security]sebs/knative/knative.py (3)
119-193: Ensurebuild_base_imagemethod handles errors and logs relevant information.The method should handle potential errors and log relevant information during the Docker image build process.
530-533: Ensurecached_functionmethod handles errors.The
cached_functionmethod should handle potential errors during logging handler updates.
306-424: Fix undefined variablefunction_cfg.The
function_cfgvariable is used before it is defined.- res = KnativeFunction( - func_name, code_package.benchmark, code_package.hash, function_cfg - ) - self.update_function(res, code_package) + function_cfg = KnativeFunctionConfig.from_benchmark(code_package) + function_cfg.storage = cast(KnativeMinio, self.get_storage()).config + function_cfg.url = function_url + res = KnativeFunction( + func_name, + code_package.benchmark, + code_package.hash, + function_cfg, + ) + self.update_function(res, code_package)Likely invalid or redundant comment.
Tools
Ruff
348-348: Local variable
languageis assigned to but never usedRemove assignment to unused variable
language(F841)
424-424: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
sebs/benchmark.py (1)
299-299: LGTM!The change to append a newline character after each package in the "requirements.txt" file improves readability and structure.
tools/knative_setup.sh (16)
1-13: Header looks good!The licensing information and brief description are correctly included.
19-23: Initialization function looks good!The
initfunction correctly initializes the necessary steps.
110-145: Main function looks good!The
mainfunction correctly orchestrates the setup process.
101-108: Version setting function looks good!The
set_versionsfunction correctly sets the versions for various components.
147-168: Function to get the latest release version looks good!The
get_latest_release_versionfunction correctly retrieves the latest version from the given repository.
170-173: Function to check if the current branch is a release branch looks good!The
is_release_branchfunction correctly determines if the current branch is a release branch.
175-185: Function to get the current branch looks good!The
current_branchfunction correctly retrieves the current branch name.
187-218: Kubernetes setup function looks good!The
kubernetesfunction correctly sets up the Kubernetes cluster.
220-236: Knative Serving setup function looks good!The
servingfunction correctly sets up Knative Serving.
238-258: DNS configuration function looks good!The
dnsfunction correctly configures DNS settings.
260-290: Load balancer setup function looks good!The
loadbalancerfunction correctly sets up the load balancer.
292-315: Networking setup function looks good!The
networkingfunction correctly sets up the ingress controller.
317-331: Knative Eventing setup function looks good!The
eventingfunction correctly sets up Knative Eventing.
333-340: Namespace creation function looks good!The
namespacefunction correctly creates and labels the namespace.
342-368: Local registry setup function looks good!The
registryfunction correctly sets up the local registry.
370-392: Next steps function looks good!The
next_stepsfunction correctly provides the next steps for managing the cluster.sebs/regression.py (3)
292-309: KnativeTestSequencePython class looks good!The class correctly extends
unittest.TestCaseand usesTestSequenceMetaas its metaclass. Theget_deploymentmethod is implemented correctly.
311-327: KnativeTestSequenceNodejs class looks good!The class correctly extends
unittest.TestCaseand usesTestSequenceMetaas its metaclass. Theget_deploymentmethod is implemented correctly.
411-420: Regression suite function looks good!The
regression_suitefunction correctly includes the new Knative test sequences.
|
@coderabbitai pause |
Actions performedReviews paused. |
Signed-off-by: Abhishek Kumar <[email protected]>
|
This current changes makes all the benchmarks run on knative platform with redeployments, Need final reviews here :)) @mcopik @oanarosca |
Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
Fully supported Knative serverless platform Integration PR cc @mcopik
Summary by CodeRabbit
New Features
systems.json.Bug Fixes
requirements.txtduring package addition.Chores
knative_setup.sh.Documentation