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

fix: typo in map schema for tolerated failure prop #133

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

massfords
Copy link
Collaborator

fix: report at most one error for max items props
feat: encode some item reader and reader config rules in map schema

fix: report at most one error for max items props
feat: encode some item reader and reader config rules in map schema
Comment on lines -54 to -58
const getPropertyCount = ({props, state}: {
const getPropertyCount = ({props, object}: {
props: string[],
state: State,
}) : number => {
return props.map((prop) => prop in state ? 1 : 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed this to work with a generic json object as opposed to assuming it's the State itself

the logic for fetching the props dynamically doesn't change

Comment on lines -63 to +74
export const AtMostOne = ({props, errorCode}:{props: string[], errorCode: StateMachineErrorCode}): StateChecker => {
export const AtMostOne = ({props, errorCode, path}: {
props: string[],
errorCode: StateMachineErrorCode,
// path to a sub-property within the state to use as the
// context for the property checks. This is intended to
// support enforcement of constraints on nested properties
// within a State.
// See the Map's ItemReader.ReaderConfiguration at most one
// rule for MaxItems and MaxItemsPath.
path?: string
}): StateChecker => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using a path param here to get the nested props keeps the rules in the validator.ts file a bit simpler

Comment on lines +38 to +40
"MaxConcurrencyPath": {
"$ref": "paths.json#/definitions/asl_ref_path"
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this property was missing

Comment on lines -39 to +43
"$comment": "Allow any object for now and update in a subsequent PR to support S3 buckets and other input sources",
"type": "object"
"$comment": "A Map State MAY have an \"ItemReader\" field, whose value MUST be a JSON object and is called the ItemReader Configuration",
"type": "object",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The ItemReader changes went in quickly a few months ago

circling back now to address this comment and flesh out the schema a bit more with the types as documented in the states language.

I included snippets of the schema text here near the relevant rules

Comment on lines -164 to +194
"ToleratedFailurePath": {
"ToleratedFailureCountPath": {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this as a copy/paste error

it was correct in the checkers but incorrect here.

@massfords massfords marked this pull request as ready for review April 17, 2023 03:35
@ChristopheBougere ChristopheBougere merged commit 1ce49e2 into ChristopheBougere:main Apr 18, 2023
@github-actions
Copy link

🎉 This PR is included in version 3.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants