Skip to content

Update vertex.py with scheduling parameters#17266

Merged
desertaxle merged 15 commits intoPrefectHQ:mainfrom
jbrache:main
Feb 28, 2025
Merged

Update vertex.py with scheduling parameters#17266
desertaxle merged 15 commits intoPrefectHQ:mainfrom
jbrache:main

Conversation

@jbrache
Copy link
Contributor

@jbrache jbrache commented Feb 24, 2025

Overview of changes

Expose the ability to specify the scheduling parameter: #15417

Checklist

Expose the ability to specify the scheduling parameter:
#15417
@github-actions github-actions bot added enhancement An improvement of an existing feature integrations Related to integrations with other services labels Feb 24, 2025
Adjusted formatting from pre-commit checks
Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jbrache! Could you please write some tests to cover this new functionality? Let me know if you need any help with that and I can provide some guidance.

@jbrache
Copy link
Contributor Author

jbrache commented Feb 24, 2025

Thanks for the PR @jbrache! Could you please write some tests to cover this new functionality? Let me know if you need any help with that and I can provide some guidance.

Hi @desertaxle yea any guidance in providing test coverage, happy to take a look at what you're expecting to see.

@desertaxle
Copy link
Member

@jbrache a test like this one would work well. You would create a new fixture containing a job configuration that uses the new scheduling parameter and assert that create_custom_job is called with the expected arguments. The test I linked is a good example of how to perform such assertions on the mocked client.

@jbrache jbrache requested a review from desertaxle February 25, 2025 04:26
@jbrache jbrache changed the title Update vertex.py Update vertex.py with scheduling parameters Feb 25, 2025
@jbrache
Copy link
Contributor Author

jbrache commented Feb 25, 2025

Added tests in test_vertex_worker.py including a new fixture containing a job configuration that uses the new scheduling parameter. I could have just re-used the existing job_config() test fixture and if you would like I can just re-use it to clean it up a bit.

Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

thanks for the PR @jbrache ! a nit and a question so far from me

Co-authored-by: nate nowack <nate@prefect.io>
Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

2 more small nits but otherwise this is looking good to me!

jbrache

This comment was marked as resolved.

@jbrache jbrache requested a review from zzstoatzz February 26, 2025 03:43
Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

thank you for the PR @jbrache !

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@desertaxle desertaxle merged commit dc25f37 into PrefectHQ:main Feb 28, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement of an existing feature integrations Related to integrations with other services

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VertexAI workpools Scheduling args Expose Vertex Dynamic Workload Scheduler on Vertex Run

3 participants