Skip to content

Enable support for Hashes in States.Hash#260

Merged
kbrock merged 1 commit into
ManageIQ:masterfrom
Fryguy:allow_hash_intrinsic_function
Aug 9, 2024
Merged

Enable support for Hashes in States.Hash#260
kbrock merged 1 commit into
ManageIQ:masterfrom
Fryguy:allow_hash_intrinsic_function

Conversation

@Fryguy
Copy link
Copy Markdown
Member

@Fryguy Fryguy commented Aug 9, 2024

While the actual hash value is different from the stepfunctions simulator it is still internally consistent, so it's usable. This would only create problems if someone ran this States.Hash in floe and tried to compare it to the exact same run in AWS, which seems unlikely. Even so, I added a pending test so we can try to figure it out later.

@kbrock or @agrare Please review. Follow up to #64

@Fryguy Fryguy requested a review from agrare as a code owner August 9, 2024 22:25
@Fryguy Fryguy added the enhancement New feature or request label Aug 9, 2024
While the actual hash value is different from the stepfunctions
simulator it is still internally consistent, so it's usable. This would
only create problems if someone ran this States.Hash in floe and tried
to compare it to the exact same run in AWS, which seems unlikely. Even
so, I added a pending test so we can try to figure it out later.
@Fryguy Fryguy force-pushed the allow_hash_intrinsic_function branch from a2c5141 to 4ca3ae6 Compare August 9, 2024 22:26
@miq-bot
Copy link
Copy Markdown
Member

miq-bot commented Aug 9, 2024

Checked commit Fryguy@4ca3ae6 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 👍

rule(:states_hash => {:args => subtree(:args)}) do
args = Transformer.process_args(args(), "States.Hash", [Object, String])
data, algorithm = *args
raise NotImplementedError if data.kind_of?(Hash)
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.

Very nice

Comment on lines -192 to -193
data = data.to_json unless data.kind_of?(String)
OpenSSL::Digest.hexdigest(algorithm, data)
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.

Really don't like these ....json... if String kind of statements

Comment on lines +504 to +505
result = described_class.value("States.Hash($.data, 'SHA-1')", {}, {"data" => {"foo" => "bar"}})
expect(result).to eq("a5e744d0164540d33b1d7ea616c28f2fa97e754a")
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.

very cool you figured this one out

@kbrock kbrock merged commit be2437c into ManageIQ:master Aug 9, 2024
agrare added a commit that referenced this pull request Aug 12, 2024
Added
- Choice rule payload validation path (#253)
- Intrinsics JsonToString and StringToJson (#256)
- Add States.Format intrinsic function (#258)
- Intrinsics States.JsonMerge (#255)
- Enable support for Hashes in States.Hash (#260)
@Fryguy Fryguy deleted the allow_hash_intrinsic_function branch August 12, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants