Skip to content

feat: add resource configuration for job #70

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.

Sign up for GitHub

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

Merged
kushsharma merged 6 commits into from
Dec 21, 2021
Merged

feat: add resource configuration for job #70

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4d92e01
feat: add resource configuration for job
irainia Dec 3, 2021
bc561b1
refactor: rename jobresource to jobspecresource for more consistency
irainia Dec 3, 2021
8459e07
chore: merge branch 'main' into optimus-job-resource
irainia Dec 8, 2021
6e54861
refactorjob resource to be put inside job metadata
irainia Dec 8, 2021
1dc74c6
chore: merge branch 'main' into optimus-job-resource
irainia Dec 17, 2021
eeaef11
refactor: rename resource configuration for better clarity
irainia Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions odpf/optimus/runtime_service.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,20 @@ message JobSpecHook {
repeated JobConfigItem config = 2;
}

message JobSpecResourceConfig {
string cpu = 1;
string memory = 2;
}

message JobSpecResource {
JobSpecResourceConfig request = 1;
JobSpecResourceConfig limit = 2;
}

@kushsharma kushsharma Dec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we prefix them with Metadata? Resource is a term already used for something else in Optimus. It doesn't directly convey the message these are execution time resource limits/requests

@irainia irainia Dec 15, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh right. Then, are JobSpecMetadataResource and JobSpecMetadataResourceConfig good enough?

@kushsharma kushsharma Dec 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah sounds good.

@irainia irainia Dec 17, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's updated as suggested


message JobMetadata {
JobSpecResource resource = 1;
}

message JobSpecification {
int32 version = 1;
string name = 2;
Expand Down Expand Up @@ -301,6 +315,8 @@ message JobSpecification {
repeated Notifiers notify = 2;
}
Behavior behavior = 19;

JobMetadata metadata = 20;
}

message JobConfigItem {
Expand Down