Skip to content

Commit

Permalink
refactor: Add skipped flag to instruction returned by the runStarlark…
Browse files Browse the repository at this point in the history
… command (#941)

## Description:
Useful in the SDK to know whether an instruction will be executed or not

## Is this change user facing?
NO
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
  • Loading branch information
Guillaume Bouvignies authored Jul 19, 2023
1 parent 4f72ae1 commit 4d88d2e
Show file tree
Hide file tree
Showing 19 changed files with 470 additions and 398 deletions.

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,13 @@ func NewStarlarkRunResponseLineFromRunSuccessEvent(serializedOutputObject string
}
}

func NewStarlarkInstruction(position *kurtosis_core_rpc_api_bindings.StarlarkInstructionPosition, name string, executableInstruction string, arguments []*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg) *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
func NewStarlarkInstruction(position *kurtosis_core_rpc_api_bindings.StarlarkInstructionPosition, name string, executableInstruction string, arguments []*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg, isSkipped bool) *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
return &kurtosis_core_rpc_api_bindings.StarlarkInstruction{
InstructionName: name,
Position: position,
ExecutableInstruction: executableInstruction,
Arguments: arguments,
IsSkipped: isSkipped,
}
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/protobuf/core/api_container_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ message StarlarkInstruction {
repeated StarlarkInstructionArg arguments = 3;

string executable_instruction = 4;

bool is_skipped = 5;
}

message StarlarkInstructionResult {
Expand Down
2 changes: 2 additions & 0 deletions api/rust/src/api_container_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ pub struct StarlarkInstruction {
pub arguments: ::prost::alloc::vec::Vec<StarlarkInstructionArg>,
#[prost(string, tag = "4")]
pub executable_instruction: ::prost::alloc::string::String,
#[prost(bool, tag = "5")]
pub is_skipped: bool,
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// Code generated by protoc-gen-grpc-web. DO NOT EDIT.
// versions:
// protoc-gen-grpc-web v1.4.2
// protoc v3.15.6
// protoc v3.19.1
// source: api_container_service.proto


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ export class StarlarkInstruction extends jspb.Message {
getExecutableInstruction(): string;
setExecutableInstruction(value: string): StarlarkInstruction;

getIsSkipped(): boolean;
setIsSkipped(value: boolean): StarlarkInstruction;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): StarlarkInstruction.AsObject;
static toObject(includeInstance: boolean, msg: StarlarkInstruction): StarlarkInstruction.AsObject;
Expand All @@ -358,6 +361,7 @@ export namespace StarlarkInstruction {
instructionName: string,
argumentsList: Array<StarlarkInstructionArg.AsObject>,
executableInstruction: string,
isSkipped: boolean,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();
var global = (function() {
if (this) { return this; }
if (typeof window !== 'undefined') { return window; }
if (typeof global !== 'undefined') { return global; }
if (typeof self !== 'undefined') { return self; }
return Function('return this')();
}.call(null));

var google_protobuf_empty_pb = require('google-protobuf/google/protobuf/empty_pb.js');
goog.object.extend(proto, google_protobuf_empty_pb);
Expand Down Expand Up @@ -3072,7 +3078,8 @@ proto.api_container_api.StarlarkInstruction.toObject = function(includeInstance,
instructionName: jspb.Message.getFieldWithDefault(msg, 2, ""),
argumentsList: jspb.Message.toObjectList(msg.getArgumentsList(),
proto.api_container_api.StarlarkInstructionArg.toObject, includeInstance),
executableInstruction: jspb.Message.getFieldWithDefault(msg, 4, "")
executableInstruction: jspb.Message.getFieldWithDefault(msg, 4, ""),
isSkipped: jspb.Message.getBooleanFieldWithDefault(msg, 5, false)
};

if (includeInstance) {
Expand Down Expand Up @@ -3127,6 +3134,10 @@ proto.api_container_api.StarlarkInstruction.deserializeBinaryFromReader = functi
var value = /** @type {string} */ (reader.readString());
msg.setExecutableInstruction(value);
break;
case 5:
var value = /** @type {boolean} */ (reader.readBool());
msg.setIsSkipped(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -3186,6 +3197,13 @@ proto.api_container_api.StarlarkInstruction.serializeBinaryToWriter = function(m
f
);
}
f = message.getIsSkipped();
if (f) {
writer.writeBool(
5,
f
);
}
};


Expand Down Expand Up @@ -3300,6 +3318,24 @@ proto.api_container_api.StarlarkInstruction.prototype.setExecutableInstruction =
};


/**
* optional bool is_skipped = 5;
* @return {boolean}
*/
proto.api_container_api.StarlarkInstruction.prototype.getIsSkipped = function() {
return /** @type {boolean} */ (jspb.Message.getBooleanFieldWithDefault(this, 5, false));
};


/**
* @param {boolean} value
* @return {!proto.api_container_api.StarlarkInstruction} returns this
*/
proto.api_container_api.StarlarkInstruction.prototype.setIsSkipped = function(value) {
return jspb.Message.setProto3BooleanField(this, 5, value);
};





Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// Code generated by protoc-gen-grpc-web. DO NOT EDIT.
// versions:
// protoc-gen-grpc-web v1.4.2
// protoc v3.15.6
// protoc v3.19.1
// source: engine_service.proto


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();
var global = (function() {
if (this) { return this; }
if (typeof window !== 'undefined') { return window; }
if (typeof global !== 'undefined') { return global; }
if (typeof self !== 'undefined') { return self; }
return Function('return this')();
}.call(null));

var google_protobuf_empty_pb = require('google-protobuf/google/protobuf/empty_pb.js');
goog.object.extend(proto, google_protobuf_empty_pb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
const (
dryRun = true
executedRun = false
isSkipped = true
)

func testInstruction() *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
Expand All @@ -25,7 +26,8 @@ func testInstruction() *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
binding_constructors.NewStarlarkInstructionArg(`["bar", "doo"]`, false),
binding_constructors.NewStarlarkInstructionKwarg("serviceA", "kwarg1", true),
binding_constructors.NewStarlarkInstructionKwarg(`struct(bonjour=42, hello="world")`, "kwarg2", false),
})
},
isSkipped)
}

func TestFormatInstruction_Executable(t *testing.T) {
Expand Down Expand Up @@ -71,7 +73,8 @@ func TestFormatInstruction_FormattingFail(t *testing.T) {
"print",
// This has issues with the quotes not being escaped
`print("UNSUPPORTED_TYPE['ModuleOutput(grafana_info=GrafanaInfo(dashboard_path="/d/QdTOwy-nz/eth2-merge-kurtosis-module-dashboard?orgId=1", user="admin", password="admin"))']")`,
[]*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg{})
[]*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg{},
isSkipped)
formattedInstruction := formatInstruction(instruction, run.Executable)
// failure to format -> the instruction is returned with no formatting applied
expectedResult := `# from dummyFile[12:4]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var (
type KurtosisInstruction interface {
GetPositionInOriginalScript() *kurtosis_starlark_framework.KurtosisBuiltinPosition

GetCanonicalInstruction() *kurtosis_core_rpc_api_bindings.StarlarkInstruction
GetCanonicalInstruction(isSkipped bool) *kurtosis_core_rpc_api_bindings.StarlarkInstruction

Execute(ctx context.Context) (*string, error)

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func newKurtosisPlanInstructionInternal(internalBuiltin *kurtosis_starlark_frame
}
}

func (builtin *kurtosisPlanInstructionInternal) GetCanonicalInstruction() *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
func (builtin *kurtosisPlanInstructionInternal) GetCanonicalInstruction(isSkipped bool) *kurtosis_core_rpc_api_bindings.StarlarkInstruction {
args := make([]*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg, len(builtin.GetArguments().GetDefinition()))
for idx, argument := range builtin.GetArguments().GetDefinition() {
name := argument.Name
Expand All @@ -44,7 +44,7 @@ func (builtin *kurtosisPlanInstructionInternal) GetCanonicalInstruction() *kurto
}
args[idx] = binding_constructors.NewStarlarkInstructionKwarg(builtin_argument.StringifyArgumentValue(value), name, shouldBeDisplayed)
}
return binding_constructors.NewStarlarkInstruction(builtin.GetPosition().ToAPIType(), builtin.GetName(), builtin.String(), args)
return binding_constructors.NewStarlarkInstruction(builtin.GetPosition().ToAPIType(), builtin.GetName(), builtin.String(), args, isSkipped)
}

// GetPositionInOriginalScript is here to implement the KurtosisInstruction interface. Remove it when it's not needed anymore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par
starlarkRunResponseLineStream <- progress

instruction := scheduledInstruction.GetInstruction()
canonicalInstruction := binding_constructors.NewStarlarkRunResponseLineFromInstruction(instruction.GetCanonicalInstruction())
canonicalInstruction := binding_constructors.NewStarlarkRunResponseLineFromInstruction(instruction.GetCanonicalInstruction(scheduledInstruction.IsExecuted()))
starlarkRunResponseLineStream <- canonicalInstruction

if !dryRun {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const (

doDryRun = true
executeForReal = false
isSkipped = false

noScriptOutputObject = ""
noParallelism = 1
Expand Down Expand Up @@ -65,9 +66,12 @@ func TestExecuteKurtosisInstructions_ExecuteForReal_Success(t *testing.T) {
require.Nil(t, err)

expectedSerializedInstructions := []*kurtosis_core_rpc_api_bindings.StarlarkInstruction{
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction3", "instruction3()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting, isSkipped),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting, isSkipped),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction3", "instruction3()", noInstructionArgsForTesting, isSkipped),
}
require.Equal(t, expectedSerializedInstructions, serializedInstruction)
require.Equal(t, executor.enclavePlan.Size(), 4) // check that the enclave plan now contains the 4 instructions
Expand Down Expand Up @@ -103,8 +107,10 @@ instruction2()

expectedSerializedInstructions := []*kurtosis_core_rpc_api_bindings.StarlarkInstruction{
// only instruction 1 and 2 because it failed at instruction 2
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting, isSkipped),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting, isSkipped),
}
require.Equal(t, expectedSerializedInstructions, serializedInstruction)
}
Expand All @@ -128,8 +134,10 @@ func TestExecuteKurtosisInstructions_DoDryRun(t *testing.T) {
require.Nil(t, err)

expectedSerializedInstructions := []*kurtosis_core_rpc_api_bindings.StarlarkInstruction{
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction1", "instruction1()", noInstructionArgsForTesting, isSkipped),
binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), "instruction2", "instruction2()", noInstructionArgsForTesting, isSkipped),
}
require.Equal(t, serializedInstruction, expectedSerializedInstructions)
}
Expand All @@ -139,8 +147,8 @@ func createMockInstruction(t *testing.T, instructionName string, executeSuccessf

stringifiedInstruction := instructionName + "()"
canonicalInstruction := binding_constructors.NewStarlarkInstruction(
dummyPosition.ToAPIType(), instructionName, stringifiedInstruction, noInstructionArgsForTesting)
instruction.EXPECT().GetCanonicalInstruction().Maybe().Return(canonicalInstruction)
dummyPosition.ToAPIType(), instructionName, stringifiedInstruction, noInstructionArgsForTesting, isSkipped)
instruction.EXPECT().GetCanonicalInstruction(mock.Anything).Maybe().Return(canonicalInstruction)
instruction.EXPECT().GetPositionInOriginalScript().Maybe().Return(dummyPosition)
instruction.EXPECT().String().Maybe().Return(stringifiedInstruction)

Expand Down
Loading

0 comments on commit 4d88d2e

Please sign in to comment.