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

rfc31: add format for the specification of job constraints #314

Closed
wants to merge 2 commits into from

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Mar 1, 2022

This is a first cut proposal for encoding generic job constraints in a JSON object for inclusion in jobspec.

The proposed format is general enough to allow fairly complex matching (in case this is ever required), but in its simplest form, looks identical to the initial proposal in flux-framework/flux-core#4143, i.e. attributes.system.constraints.properties=[ array of required properties ].

The format is largely inspired by JsonLogic.

I decided it would be cleaner to propose a separate RFC for job constraints rather than adding separately to each of the jobspec RFCs. We can then refer to this RFC when describing where constraints go in the jobspec RFCs, as we have done with dependencies.

Fixes #243
Fixes #246

@grondo grondo changed the title RFC 31: add format for the specification of job constraints rfc31: add format for the specification of job constraints Mar 1, 2022
@grondo grondo requested a review from dongahn March 1, 2022 22:28
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

decided it would be cleaner to propose a separate RFC for job constraints rather than adding separately to each of the jobspec RFCs. We can then refer to this RFC when describing where constraints go in the jobspec RFCs, as we have done with dependencies.

I think that was the right move. This looks good as is!

@grondo
Copy link
Contributor Author

grondo commented Mar 11, 2022

@garlick noticed the commit here was accidentally included in the merged PR #313.

I'll rework this PR to accumulate any amendments (somehow), or maybe close it if the end result was acceptable.

Sorry about that!

grondo added 2 commits March 11, 2022 07:31
This reverts commit d3139d2.

Revert accidentally committed RFC 31 so it can be proposed in its
own PR.
Problem: There is no defintion of how to encode job constraints in
a standardized format.

Introduce RFC 31: Job Constraints Specification, which includes a simple,
but extensible format for encoding job constraints, inspired by the
json-logic definition at https://jsonlogic.com.
@grondo
Copy link
Contributor Author

grondo commented Mar 11, 2022

Ok, what I've done is revert the accidental commit and then committed it again immediately on top so that it can properly be merged in this PR.

If the original was acceptable we could optionally close this PR and make changes in future PRs.

@dongahn
Copy link
Member

dongahn commented Mar 16, 2022

attributes.system.constraints.properties

We have used attributes.system.scheduler.queue to capture queue name constraints. I have a slight preference to use scheduler key instead of constraints. That way, all of the scheduler's constraints can be found under one parent key.

Having said that, do you see a user case of properties beyond scheduler constraints? In case you have a specific reason to use constraints as your parent key choice.

@grondo
Copy link
Contributor Author

grondo commented Mar 16, 2022

Here is my main argument against attributes.system.scheduler.constraints.properties:

Since the main purpose of jobspec is to express program requirements to the scheduler, I find a separate scheduler key to be a slight redundancy -- though for scheduler specific, non-RFC 14 data, it does make sense to adopt a scheduler key where the contents are opaque to flux-core.

Since constraints will be standardized via an RFC, it thus makes sense to promote the key to the top level of jobspec system attributes. This will make it easier (i.e. less verbose) for flux-core utilities to generate and query jobspec constraints, and easier for users crafting jobspec with constraints.

@dongahn
Copy link
Member

dongahn commented Mar 16, 2022

@grondo: that's a good argument. Keeping generic properties at a higher level and scheduler-specific parameters under scheduler key. I am sold on that then.

@grondo
Copy link
Contributor Author

grondo commented Apr 5, 2022

closing this one since RFC 31 was merged in a separate PR

@grondo grondo closed this Apr 5, 2022
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.

RFC25: Need bare minimum support for properties spec RCF14: add property support into V1 jobspec.
3 participants