Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Optional override for Vertex custom job name #208

Merged
merged 31 commits into from
Sep 19, 2023

Conversation

jeremy-thomas-roc
Copy link
Contributor

@jeremy-thomas-roc jeremy-thomas-roc commented Sep 1, 2023

Add optional parameter to VertexAICustomJob to name jobs. This helps in monorepo situations where all Vertex jobs will just be {repo-name}-{uuid}, which is not very helpful when running lots of scheduled jobs.

Screenshots

Checklist

  • References any related issue by including "Closes #" or "Closes ".
    • If no issue exists and your change is not a small fix, please create an issue first.
  • Includes tests or only affects documentation.
  • Passes pre-commit checks.
    • Run pre-commit install && pre-commit run --all locally for formatting and linting.
  • Includes screenshots of documentation updates.
    • Run mkdocs serve view documentation locally.
  • Summarizes PR's changes in CHANGELOG.md

jeremy-thomas-roc and others added 24 commits April 24, 2023 08:58
…tHQ#196)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/jamesives/github-pages-deploy-action) from 4.4.2 to 4.4.3.
- [Release notes](https://github.com/jamesives/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@v4.4.2...v4.4.3)

---
updated-dependencies:
- dependency-name: JamesIves/github-pages-deploy-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alexander Streed <[email protected]>
* Persist Labels to Vertex AI Custom Job

* ensure compatible with GCP requirements

* small bug not fetching key-value pairs

* Add test, changelog, and format

* pre commit hook changes

* Feedback for method name

---------

Co-authored-by: Alexander Streed <[email protected]>
@jeremy-thomas-roc jeremy-thomas-roc requested a review from a team as a code owner September 1, 2023 13:52
@jeremy-thomas-roc
Copy link
Contributor Author

@desertaxle Can I get some feedback on this? I believe the failing tests happened when there was a dependency issue of the prefect side, but I am unable to re-run them to verify.

@desertaxle
Copy link
Member

@jeremy-thomas-roc I think we could use the existing name attribute on the VertexAICustomJob job to create more unique names for created jobs. By default, name will be the flow run name, which has the added advantage of making it easier to map Vertex AI jobs to flow runs. Let me know if that would work for your use case or it you have any questions on implementation.

@jeremy-thomas-roc
Copy link
Contributor Author

jeremy-thomas-roc commented Sep 18, 2023

@desertaxle that works for me, implemented that change

It appears that name can be None when this property is retrieved, which I see from one of the failing tests. Is this an issue where the name is not created before the job_name?

@desertaxle
Copy link
Member

@jeremy-thomas-roc That's true that name can be None if the block is used outside of a deployment. A simple check to see if name is set should work. We do something similar in prefect-azure. You can see PrefectHQ/prefect-azure#92 as an example.

@jeremy-thomas-roc
Copy link
Contributor Author

@desertaxle I have made that change, but it appears an unrelated test is failing due to an API error, not sure why that is happening

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.

Couple of small nits, but otherwise this is good to go!

CHANGELOG.md Outdated
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

### Changed
- `job_name` in `VertexAICustomJob` is now based on the flow run name instead of the image repository.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `job_name` in `VertexAICustomJob` is now based on the flow run name instead of the image repository.
- Use flow run name for name of created custom jobs - [#208](https://github.com/PrefectHQ/prefect-gcp/pull/208)

Comment on lines 202 to 203
repo_name = self.name or self.image.split("/")[2]
return f"{repo_name}-{uuid4().hex}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
repo_name = self.name or self.image.split("/")[2]
return f"{repo_name}-{uuid4().hex}"
base_name = self.name or self.image.split("/")[2]
return f"{base_name}-{uuid4().hex}"

@jeremy-thomas-roc
Copy link
Contributor Author

@desertaxle suggested changes implemented

@desertaxle desertaxle merged commit dfa07ca into PrefectHQ:main Sep 19, 2023
10 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants