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

jobspec: make max/operator/operand optional #879

Merged
merged 2 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 15 additions & 20 deletions resource/libjobspec/jobspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ parse_error::parse_error(const YAML::Node &node, const char *msg)
namespace {
void parse_yaml_count (Resource& res, const YAML::Node &cnode)
{
/* count can have an unsigned interger value */
/* count can have an unsigned integer value */
if (cnode.IsScalar()) {
res.count.min = cnode.as<unsigned>();
res.count.max = res.count.min;
Expand All @@ -57,50 +57,47 @@ void parse_yaml_count (Resource& res, const YAML::Node &cnode)
throw parse_error (cnode, "count is not a mapping");
}

/* Verify existance of required entries */
/* Verify existence of required entries */
if (!cnode["min"]) {
throw parse_error (cnode, "Key \"min\" missing from count");
}
if (!cnode["min"].IsScalar()) {
throw parse_error (cnode["min"], "Value of \"min\" must be a scalar");
}
if (!cnode["max"]) {
throw parse_error (cnode, "Key \"max\" missing from count");
}
if (!cnode["max"].IsScalar()) {
if (cnode["max"] && !cnode["max"].IsScalar()) {
Copy link
Member

Choose a reason for hiding this comment

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

Nits: you may want to fix the "interger" typo in line 48 and "existance" in line 60 above.

Copy link
Member Author

Choose a reason for hiding this comment

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

:-). Great catch, human valgrind! They must have been in there from day 1. I will do a fix up commit.

throw parse_error (cnode["max"], "Value of \"max\" must be a scalar");
}
if (!cnode["operator"]) {
throw parse_error (cnode, "Key \"operator\" missing from count");
}
if (!cnode["operator"].IsScalar()) {
if (cnode["operator"] && !cnode["operator"].IsScalar()) {
throw parse_error (cnode["operator"],
"Value of \"operator\" must be a scalar");
}
if (!cnode["operand"]) {
throw parse_error (cnode, "Key \"operand\" missing from count");
}
if (!cnode["operand"].IsScalar()) {
if (cnode["operand"] && !cnode["operand"].IsScalar()) {
throw parse_error (cnode["operand"],
"Value of \"operand\" must be a scalar");
}

/* Validate values of entries */
res.count.min = cnode["min"].as<unsigned>();
if (cnode["max"]) {
res.count.max = cnode["max"].as<unsigned>();
}
if (cnode["operator"]) {
res.count.oper = cnode["operator"].as<char>();
}
if (cnode["operand"]) {
res.count.operand = cnode["operand"].as<int>();
}

if (res.count.min < 1) {
throw parse_error (cnode["min"], "\"min\" must be greater than zero");
}

res.count.max = cnode["max"].as<unsigned>();
if (res.count.max < 1) {
throw parse_error (cnode["max"], "\"max\" must be greater than zero");
}
if (res.count.max < res.count.min) {
throw parse_error (cnode["max"],
"\"max\" must be greater than or equal to \"min\"");
}

res.count.oper = cnode["operator"].as<char>();
switch (res.count.oper) {
case '+':
case '*':
Expand All @@ -109,8 +106,6 @@ void parse_yaml_count (Resource& res, const YAML::Node &cnode)
default:
throw parse_error (cnode["operator"], "Invalid count operator");
}

res.count.operand = cnode["operand"].as<int>();
}
}

Expand Down
3 changes: 2 additions & 1 deletion resource/libjobspec/jobspec.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <stdexcept>
#include <unordered_map>
#include <cstdint>
#include <limits>
#include <yaml-cpp/yaml.h>

namespace Flux {
Expand All @@ -55,7 +56,7 @@ class Resource {
std::string type;
struct {
unsigned min;
unsigned max;
unsigned max = std::numeric_limits<unsigned int>::max();
char oper = '+';
int operand = 1;
} count;
Expand Down