Skip to content
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

parse queue name #857

Merged
merged 2 commits into from Jan 26, 2023
Merged

Conversation

Anton-Cherepkov
Copy link
Contributor

@Anton-Cherepkov Anton-Cherepkov commented Dec 20, 2022

Related Issue \ discussion

https://clearml.slack.com/archives/CTK20V944/p1671538941447289

Patch Description

Now one may specify things like ${pipeline.queue_name} for queue_name and control the execution queue of each task via pipeline parameters.

Testing Instructions

Other Information

@TezRomacH
Copy link

Wow! Really useful feature!

return node.job.launch(queue_name=node.queue or self._default_execution_queue)

parsed_queue_name = self._parse_step_ref(node.queue)
return node.job.launch(parsed_queue_name or self._default_execution_queue)
Copy link
Member

Choose a reason for hiding this comment

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

@Anton-Cherepkov just like in the previous diff, I think it would be better to keep the previous keyword calling style (i.e. queue_name=...)

Copy link
Member

Choose a reason for hiding this comment

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

@Anton-Cherepkov can you please change this to:
return node.job.launch(queue_name=parsed_queue_name or self._default_execution_queue)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkhenning yes, sure. 04165b7

keyword queue_name
@jkhenning jkhenning merged commit 54c601e into allegroai:master Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants