Pipeline generator utils #31
Conversation
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
Signed-off-by: kevin <[email protected]>
|
|
||
|
|
||
| class AgentQueue(str, enum.Enum): | ||
| AWS_CPU = "cpu_queue" |
There was a problem hiding this comment.
why some have the _queue suffix and some do not?
There was a problem hiding this comment.
the ones with _queue are those that we name it ourselves.. the one without are from AMD ..
scripts/pipeline_generator/utils.py
Outdated
| def get_full_test_command(test_commands: List[str], step_working_dir: str) -> str: | ||
| """Convert test commands into one-line command with the right directory.""" | ||
| working_dir = step_working_dir or DEFAULT_WORKING_DIR | ||
| test_commands_str = "; ".join(test_commands) |
There was a problem hiding this comment.
I am still a bit worried that this is not the write way. maybe concating with end-lines will be safer.
scripts/pipeline_generator/utils.py
Outdated
| """Convert test commands into one-line command with the right directory.""" | ||
| working_dir = step_working_dir or DEFAULT_WORKING_DIR | ||
| test_commands_str = "; ".join(test_commands) | ||
| return f"cd {working_dir}; {test_commands_str}" |
There was a problem hiding this comment.
why do you need to add cd working_dir here? the docker plugin has builtin workdir field?
There was a problem hiding this comment.
I'm not sure if kubernetes plugin has it ... this will be used to generate command for both k8s & docker plugin
There was a problem hiding this comment.
it might work with workingDir in the k8s podspec.. I can try it out after the pipeline generator is up and if it works, remove cd .. from the command and specify working dir in the plugin
Signed-off-by: kevin <[email protected]>
| test_commands_str = ";\n".join(test_commands) | ||
| return f"cd {working_dir};\n{test_commands_str}" |
There was a problem hiding this comment.
if you added the \n, do you still need to add the ; ?
There was a problem hiding this comment.
probably not but let's just leave it there for now... I can test again & take it out later when it's up and running
No description provided.