Skip to content

[Relay] Bug fix when applying history using an iterator or records.#11306

Merged
jwfromm merged 2 commits into
apache:mainfrom
jwfromm:log_load_iter_fix
May 17, 2022
Merged

[Relay] Bug fix when applying history using an iterator or records.#11306
jwfromm merged 2 commits into
apache:mainfrom
jwfromm:log_load_iter_fix

Conversation

@jwfromm

@jwfromm jwfromm commented May 13, 2022

Copy link
Copy Markdown
Contributor

I introduced a bug in #11199 that causes an error when an Iterator of records is used as the argument to ApplyHistoryBest. This small PR fixes it by more carefully checking input types.

@junrushao junrushao left a comment

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.

LGTM

@jwfromm

jwfromm commented May 13, 2022

Copy link
Copy Markdown
Contributor Author

@mbs-octoml this should fix the issue you ran into. I decided to go with more careful type checking rather than changing the append since I think this is a more intuitive way of handling both lists of (Input, Records) pairs and iterators over them.

@mbs-octoml mbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. String is Iterable!

@jwfromm jwfromm merged commit 75c31ca into apache:main May 17, 2022
@jwfromm jwfromm deleted the log_load_iter_fix branch April 12, 2023 15:55
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.

3 participants