-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: Knative deployment support #206
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
KnativeMinio
class 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 thedeserialize
method.Adding a docstring for the
deserialize
method 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 | Y
for type annotations.To follow modern Python type annotation practices, use
X | Y
instead ofOptional[X]
.- storage: Optional[MinioConfig] = None + storage: MinioConfig | None = NoneTools
Ruff
15-15: Use
X | Y
for type annotationsConvert to
X | Y
(UP007)
18-25
: Consider adding a docstring for thedeserialize
method.Adding a docstring for the
deserialize
method 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 theserialize
method.Adding a docstring for the
serialize
method 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_benchmark
method.Adding a docstring for the
from_benchmark
method 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 theconfig
property.Adding a docstring for the
config
property can help clarify its functionality.""" Get the KnativeFunctionConfig instance associated with this function. Returns: - KnativeFunctionConfig instance. """
50-52
: Consider adding a docstring for thetypename
method.Adding a docstring for the
typename
method 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 theserialize
method.Adding a docstring for the
serialize
method 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 thedeserialize
method.Adding a docstring for the
deserialize
method 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_command
is confusing as it returns a list of arguments, not a command.Consider renaming it to
get_arguments
for better clarity.
54-54
: Use f-string instead offormat
call.Convert the
format
call 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
format
callConvert 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 | Y
for type annotationsConvert to
X | Y
(UP007)
sebs/knative/triggers.py
5-5:
typing.Dict
imported but unusedRemove unused import:
typing.Dict
(F401)
40-45: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
54-54: Use f-string instead of
format
callConvert to f-string
(UP032)
sebs/faas/config.py
213-213: Use
config.get(name, config)
instead of anif
blockReplace with
config.get(name, config)
(SIM401)
sebs/knative/config.py
2-2:
sebs.faas.config.Credentials
imported but unusedRemove unused import:
sebs.faas.config.Credentials
(F401)
sebs/knative/knative.py
1-1:
code
imported but unusedRemove unused import:
code
(F401)
2-2:
os.devnull
imported but unusedRemove unused import:
os.devnull
(F401)
4-4:
flask.config
imported but unusedRemove unused import:
flask.config
(F401)
6-6:
sebs.faas.function.ExecutionResult
imported but unusedRemove unused import:
sebs.faas.function.ExecutionResult
(F401)
13-13:
sebs.knative.triggers.KnativeLibraryTrigger
imported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger
(F401)
14-14:
sebs.faas.config.Resources
imported but unusedRemove unused import:
sebs.faas.config.Resources
(F401)
15-15:
typing.Dict
imported but unusedRemove unused import
(F401)
15-15:
typing.Optional
imported but unusedRemove unused import
(F401)
18-18:
uuid
imported but unusedRemove unused import:
uuid
(F401)
30-30: Redefinition of unused
config
from line 4(F811)
53-53: Redefinition of unused
config
from line 4(F811)
149-154: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
158-158: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
222-222: Undefined name
function_cfg
(F821)
232-237: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
240-245: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
255-260: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
286-286: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
292-292: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
305-319: Prefer
capture_output
over sendingstdout
andstderr
toPIPE
Replace with
capture_output
keyword argument(UP022)
326-326: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
333-333: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
352-352: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Additional comments not posted (9)
tools/knative_preparation.py (1)
11-12
: LGTM!The function
is_installed
is well-structured and there are no issues.sebs/knative/knative.py (7)
52-55
: LGTM!The
config
method correctly returns the Knative configuration.Tools
Ruff
53-53: Redefinition of unused
config
from line 4(F811)
57-59
: LGTM!The
get_knative_func_cmd
method correctly returns the Knative function command.
61-64
: LGTM!The
function_type
method correctly returns the specific function type for Knative.
71-86
: LGTM!The
find_image
method correctly checks if a Docker image exists in the repository.
166-200
: Ensure the image tag includesknative
.The image tag should include
knative
to avoid mixing images between OpenWhisk and Knative.Ensure that the image tag includes
knative
.
364-365
: LGTM!The
name
method correctly returns the name of the system.
27-37
: Avoid redefinition of theconfig
parameter.The
config
parameter is redefined, which can cause confusion and potential issues.- from flask import config
Likely invalid or redundant comment.
Tools
Ruff
30-30: Redefinition of unused
config
from 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:
json
imported but unusedRemove unused import:
json
(F401)
Additional comments not posted (7)
benchmarks/wrappers/knative/python/storage.py (7)
42-48
: LGTM!The
unique_name
method correctly generates a unique name for a file.
51-54
: LGTM!The
upload
method correctly handles file uploads and returns the unique key name.
56-57
: LGTM!The
download
method correctly handles file downloads.
59-63
: LGTM!The
download_directory
method correctly handles directory downloads.
65-70
: LGTM!The
upload_stream
method correctly handles stream uploads and returns the unique key name.
72-74
: LGTM!The
download_stream
method correctly handles stream downloads.
76-80
: LGTM!The
get_instance
method 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 thehandler
call for accuracy.To ensure accurate timing, move the
begin
andend
variables closer to thehandler
call.- 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 thehandler
call for accuracy.To ensure accurate timing, move the
begin
andend
variables closer to thehandler
call.- 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 thehandler
call for accuracy.To ensure accurate timing, move the
startTime
andendTime
variables closer to thehandler
call.- 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 thehandler
call for accuracy.To ensure accurate timing, move the
startTime
andendTime
variables closer to thehandler
call.- 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=True
for immutability.Consider adding
frozen=True
to the@dataclass
decorator to make instances of this class immutable.- @dataclass + @dataclass(frozen=True)
30-33
: Remove unnecessarysuper
call.The
super
call 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
: Uselogging
instead ofUsing the
logging
module 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
MinioConfig
import is unused and should be removed.- from sebs.storage.config import MinioConfig
53-53
: Remove unused local variable.The
cached_config
variable is assigned but never used.- cached_config = cache.get_config("knative")
191-191
: Remove unused local variable.The
cached_config
variable is assigned but never used.- cached_config = cache.get_config("knative")
Tools
Ruff
191-191: Local variable
cached_config
is assigned to but never usedRemove assignment to unused variable
cached_config
(F841)
sebs/knative/knative.py (8)
13-13
: Remove unused import.The
KnativeLibraryTrigger
class 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.KnativeLibraryTrigger
imported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger
(F401)
14-14
: Remove unused importstyping.Dict
andtyping.Optional
.The
Dict
andOptional
types 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.Dict
imported but unusedRemove unused import
(F401)
14-14:
typing.Optional
imported but unusedRemove unused import
(F401)
18-18
: Remove redefined import.The
KnativeMinio
import is redefined and should be removed.- from sebs.knative.storage import KnativeMinio
Tools
Ruff
18-18: Redefinition of unused
KnativeMinio
from line 12Remove definition:
KnativeMinio
(F811)
206-206
: Remove unused local variable.The
package_config
variable is assigned but never used.- package_config = CONFIG_FILES[language_name]
Tools
Ruff
206-206: Local variable
package_config
is assigned to but never usedRemove assignment to unused variable
package_config
(F841)
263-263
: Remove unused local variable.The
language
variable is assigned but never used.- language = code_package.language_name
Tools
Ruff
263-263: Local variable
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
316-316
: Raise exceptions withraise ... from err
orraise ... from None
.Exceptions should be raised with
raise ... from err
orraise ... from None
to 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
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
356-356
: Raise exceptions withraise ... from err
orraise ... from None
.Exceptions should be raised with
raise ... from err
orraise ... from None
to distinguish them from errors in exception handling.- raise RuntimeError(e) + raise RuntimeError(e) from eTools
Ruff
356-356: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
374-374
: Raise exceptions withraise ... from err
orraise ... from None
.Exceptions should be raised with
raise ... from err
orraise ... from None
to distinguish them from errors in exception handling.- raise RuntimeError(e) + raise RuntimeError(e) from eTools
Ruff
374-374: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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:
json
imported but unusedRemove unused import:
json
(F401)
sebs/knative/config.py
191-191: Local variable
cached_config
is assigned to but never usedRemove assignment to unused variable
cached_config
(F841)
sebs/knative/knative.py
13-13:
sebs.knative.triggers.KnativeLibraryTrigger
imported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger
(F401)
14-14:
typing.Dict
imported but unusedRemove unused import
(F401)
14-14:
typing.Optional
imported but unusedRemove unused import
(F401)
18-18: Redefinition of unused
KnativeMinio
from line 12Remove definition:
KnativeMinio
(F811)
206-206: Local variable
package_config
is assigned to but never usedRemove assignment to unused variable
package_config
(F841)
256-256: Undefined name
function_cfg
(F821)
263-263: Local variable
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
316-316: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
356-356: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
374-374: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Additional comments not posted (19)
sebs/knative/storage.py (3)
10-11
: LGTM!The
deployment_name
method is simple and correct.
13-20
: LGTM!The
__init__
method correctly initializes theKnativeMinio
class.
23-28
: LGTM!The
deserialize
method 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_name
method is correctly implemented.
33-36
: LGTM!The
upload
method is correctly implemented.
38-40
: LGTM!The
download
method is correctly implemented.
42-47
: LGTM!The
uploadStream
method is correctly implemented.
49-51
: LGTM!The
downloadStream
method 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
json
module is imported but not used in the file.- import json
Tools
Ruff
3-3:
json
imported 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_installed
is 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.KnativeLibraryTrigger
imported but unusedRemove unused import:
sebs.knative.triggers.KnativeLibraryTrigger
(F401)
14-14:
typing.Dict
imported but unusedRemove unused import
(F401)
14-14:
typing.Optional
imported but unusedRemove unused import
(F401)
18-18: Redefinition of unused
KnativeMinio
from line 12Remove definition:
KnativeMinio
(F811)
206-206: Local variable
package_config
is assigned to but never usedRemove assignment to unused variable
package_config
(F841)
256-256: Undefined name
function_cfg
(F821)
263-263: Local variable
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
316-316: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
356-356: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
374-374: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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_output
instead of sendingstdout
andstderr
toPIPE
.The
capture_output
argument 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
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
318-357
: Usecapture_output
instead of sendingstdout
andstderr
toPIPE
.The
capture_output
argument 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
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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_command
has error handling.The function correctly installs Minikube if it is not already installed. Ensure
run_command
has proper error handling as suggested.
26-34
: LGTM! Ensurerun_command
has error handling.The function correctly installs kubectl if it is not already installed. Ensure
run_command
has proper error handling as suggested.
36-43
: LGTM! Ensurerun_command
has error handling.The function correctly installs Cosign if it is not already installed. Ensure
run_command
has 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_config
is 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_FILES
is assigned to but never usedRemove assignment to unused variable
CONFIG_FILES
(F841)
294-294: Local variable
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
359-359: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
395-395: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
399-399: Local variable
docker_image
is assigned to but never usedRemove assignment to unused variable
docker_image
(F841)
419-419: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
424-424: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
443-443: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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_bucket
parameter is utilized if needed.The parameter
benchmarks_bucket
is 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_bucket
parameter is utilized if needed.The parameter
benchmarks_bucket
is 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_bucket
parameter is utilized if needed.The parameter
benchmarks_bucket
is 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_bucket
parameter is utilized if needed.The parameter
benchmarks_bucket
is 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.json
includes the necessary properties and values forpython
andnodejs
languages. It is consistent with the configurations of other systems like AWS, Azure, GCP, and OpenWhisk. Each configuration block includesbase_images
,images
,username
, anddeployment
(withfiles
andpackages
), which aligns with the patterns observed in other systems.
Knative Configuration:
- Python: Includes
base_images
,images
,username
, anddeployment
(withfiles
andpackages
).- Nodejs: Includes
base_images
,images
,username
, anddeployment
(withfiles
andpackages
).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_knative
correctly sanitizes benchmark names for Knative.sebs/knative/config.py (3)
9-27
: LGTM!The class
KnativeCredentials
correctly handles Docker credentials for Knative.
30-173
: LGTM!The class
KnativeResources
correctly handles Docker registry and Minio storage configurations for Knative.
176-227
: LGTM!The class
KnativeConfig
correctly handles the overall configuration for Knative, including resources and credentials.Tools
Ruff
213-213: Local variable
cached_config
is 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 theKnative
object and handles Docker login if credentials are provided.
49-51
: LGTM!The
initialize
method correctly callsinitialize_resources
with 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_storage
method correctly retrieves or initializes Knative Minio storage based on the provided configuration.
73-80
: LGTM!The
shutdown
method correctly stops storage if required and deletes the Knative cluster if configured.
115-189
: LGTM!The
build_base_image
method correctly builds the base image for the function using the 'func build' command.
454-457
: LGTM!The
cached_function
method correctly sets logging handlers for HTTP triggers of a function.
458-462
: LGTM!The
default_function_name
method correctly generates a default function name based on the benchmark and language details.
361-396
: Usecapture_output
instead of sendingstdout
andstderr
toPIPE
.The
capture_output
argument 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
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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
KUBECONFIG
path should specify the exact location of thebin
folder.- 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.jsonify
imported but unusedRemove unused import:
flask.jsonify
(F401)
7-7:
minio
imported but unusedRemove unused import:
minio
(F401)
benchmarks/wrappers/knative/python/storage.py
3-3:
json
imported but unusedRemove unused import:
json
(F401)
sebs/knative/config.py
215-215: Local variable
cached_config
is 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
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
424-424: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
460-460: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
466-466: Local variable
docker_image
is assigned to but never usedRemove assignment to unused variable
docker_image
(F841)
488-488: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
495-495: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
519-519: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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
upload
method 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
KnativeCredentials
class stores sensitive data likedocker_username
anddocker_password
. Ensure these are handled securely.
[security]
32-43
: Ensure sensitive data is handled securely.The
KnativeResources
class stores sensitive data likedocker_username
anddocker_password
. Ensure these are handled securely.
[security]
182-189
: Ensure sensitive data is handled securely.The
KnativeConfig
class stores sensitive data likeknativeExec
,shutdownStorage
, andremoveCluster
. Ensure these are handled securely.
[security]sebs/knative/knative.py (3)
119-193
: Ensurebuild_base_image
method 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_function
method handles errors.The
cached_function
method should handle potential errors during logging handler updates.
306-424
: Fix undefined variablefunction_cfg
.The
function_cfg
variable 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
language
is assigned to but never usedRemove assignment to unused variable
language
(F841)
424-424: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to 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
init
function correctly initializes the necessary steps.
110-145
: Main function looks good!The
main
function correctly orchestrates the setup process.
101-108
: Version setting function looks good!The
set_versions
function correctly sets the versions for various components.
147-168
: Function to get the latest release version looks good!The
get_latest_release_version
function 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_branch
function correctly determines if the current branch is a release branch.
175-185
: Function to get the current branch looks good!The
current_branch
function correctly retrieves the current branch name.
187-218
: Kubernetes setup function looks good!The
kubernetes
function correctly sets up the Kubernetes cluster.
220-236
: Knative Serving setup function looks good!The
serving
function correctly sets up Knative Serving.
238-258
: DNS configuration function looks good!The
dns
function correctly configures DNS settings.
260-290
: Load balancer setup function looks good!The
loadbalancer
function correctly sets up the load balancer.
292-315
: Networking setup function looks good!The
networking
function correctly sets up the ingress controller.
317-331
: Knative Eventing setup function looks good!The
eventing
function correctly sets up Knative Eventing.
333-340
: Namespace creation function looks good!The
namespace
function correctly creates and labels the namespace.
342-368
: Local registry setup function looks good!The
registry
function correctly sets up the local registry.
370-392
: Next steps function looks good!The
next_steps
function correctly provides the next steps for managing the cluster.sebs/regression.py (3)
292-309
: KnativeTestSequencePython class looks good!The class correctly extends
unittest.TestCase
and usesTestSequenceMeta
as its metaclass. Theget_deployment
method is implemented correctly.
311-327
: KnativeTestSequenceNodejs class looks good!The class correctly extends
unittest.TestCase
and usesTestSequenceMeta
as its metaclass. Theget_deployment
method is implemented correctly.
411-420
: Regression suite function looks good!The
regression_suite
function 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.txt
during package addition.Chores
knative_setup.sh
.Documentation