Skip to content

Add empty list check for element before table to fix "list index out of range"#549

Merged
RobotSail merged 1 commit intoinstructlab:mainfrom
cfchase:fix-table-chunker
Feb 21, 2025
Merged

Add empty list check for element before table to fix "list index out of range"#549
RobotSail merged 1 commit intoinstructlab:mainfrom
cfchase:fix-table-chunker

Conversation

@cfchase
Copy link
Contributor

@cfchase cfchase commented Feb 11, 2025

Fixes #548

@mergify mergify bot added the ci-failure label Feb 11, 2025
@aakankshaduggal aakankshaduggal requested a review from a team February 11, 2025 18:30
aakankshaduggal
aakankshaduggal approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@aakankshaduggal aakankshaduggal left a comment

Choose a reason for hiding this comment

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

LGTM thanks @cfchase

@aakankshaduggal aakankshaduggal requested a review from a team February 11, 2025 18:30
@mergify mergify bot added the one-approval label Feb 11, 2025
RobotSail
RobotSail approved these changes Feb 11, 2025
View reviewed changes
Copy link
Member

@RobotSail RobotSail left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot removed the one-approval label Feb 11, 2025
@khaledsulayman khaledsulayman added the hold In-progress PR. Tag should be removed before merge. label Feb 11, 2025
@khaledsulayman
Copy link
Contributor

khaledsulayman commented Feb 11, 2025

placing a hold until I verify that I've been able to reproduce.

also @cfchase looks like you need to sign-off your commits

khaledsulayman
khaledsulayman approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@khaledsulayman khaledsulayman left a comment

Choose a reason for hiding this comment

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

Thanks for finding and submitting this fix!

@khaledsulayman
Copy link
Contributor

khaledsulayman commented Feb 11, 2025

looks like the medium job failed due to insufficient space. Just reran it for now and can investigate further if it fails again

@mergify mergify bot added ci-failure and removed ci-failure labels Feb 11, 2025
@Prithiviraj25
Copy link

Prithiviraj25 commented Feb 12, 2025

failed to generate data with exception: 'icl_query_2' can you help me out like why does this error arise?

@bbrowning
Copy link
Contributor

bbrowning commented Feb 12, 2025

@Prithiviraj25 Is your comment and issue about the changes in this pull request? If so, can you share how you hit that error with this PR? If not, can you open another issue specifically to track your error?

@khaledsulayman khaledsulayman removed the hold In-progress PR. Tag should be removed before merge. label Feb 12, 2025
@Prithiviraj25
Copy link

Prithiviraj25 commented Feb 12, 2025

@Prithiviraj25 Is your comment and issue about the changes in this pull request? If so, can you share how you hit that error with this PR? If not, can you open another issue specifically to track your error?
Its not related to this. Anyways thanks i ll open another issue

@khaledsulayman
Copy link
Contributor

khaledsulayman commented Feb 12, 2025

removed the hold since I was able to reproduce the error, however looks like there's some failing functional tests

@khaledsulayman
Copy link
Contributor

khaledsulayman commented Feb 12, 2025

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Feb 12, 2025

refresh

✅ Pull request refreshed

@khaledsulayman
Copy link
Contributor

khaledsulayman commented Feb 12, 2025

@Mergifyio retest

@mergify
Copy link
Contributor

mergify bot commented Feb 12, 2025

retest

❌ Sorry but I didn't understand the command. Please consult the commands documentation 📚.

@jjasghar
Copy link

jjasghar commented Feb 13, 2025

I ran this in the cmb builder, and it got me past the issue we had with the list index out range index error.

@nimbinatus nimbinatus mentioned this pull request Feb 13, 2025
Closed
@RobotSail
Copy link
Member

RobotSail commented Feb 17, 2025

@mergify rebase

@cfchase @RobotSail
Add empty list check for element before table to fix "list index out …
eb63902 
…of range"

Signed-off-by: Chris Chase <cchase@redhat.com>
@mergify
Copy link
Contributor

mergify bot commented Feb 17, 2025

rebase

✅ Branch has been successfully rebased

@mergify mergify bot added ci-failure and removed ci-failure labels Feb 17, 2025
@cfchase
Copy link
Contributor Author

cfchase commented Feb 21, 2025

@mergify rebase

@mergify
Copy link
Contributor

mergify bot commented Feb 21, 2025

rebase

❌ Unable to rebase: user cfchase is unknown.

Details

Please make sure cfchase has logged in Mergify dashboard.

@cfchase
Copy link
Contributor Author

cfchase commented Feb 21, 2025

@mergify rebase

@mergify
Copy link
Contributor

mergify bot commented Feb 21, 2025

rebase

❌ Base branch update has failed

Details

cfchase does not have write access to the forked repository.

@RobotSail RobotSail merged commit e116e8d into instructlab:main Feb 21, 2025
27 of 28 checks passed
@RobotSail
Copy link
Member

RobotSail commented Feb 21, 2025

Merging this into main since the change being made here is only adding a safety precaution before we access a potentially undefined variable, and many people have complained about this exact issue. The e2e test failure is unrelated.

@ktdreyer
Copy link
Contributor

ktdreyer commented Mar 13, 2025

@bbrowning are you ok running @mergify rebase release-0.7 here in order to backport this to our latest stable release series?

@nathan-weinberg
Copy link
Member

nathan-weinberg commented Mar 13, 2025

@bbrowning are you ok running @mergify rebase release-0.7 here in order to backport this to our latest stable release series?

I think this should be @Mergifyio backport release-0.7

@bbrowning
Copy link
Contributor

bbrowning commented Mar 13, 2025

@Mergifyio backport release-0.7

@mergify
Copy link
Contributor

mergify bot commented Mar 13, 2025

backport release-0.7

❌ No backport have been created

Details
  • Backport to branch release-0.7 failed

GitHub error: Branch not found

@bbrowning
Copy link
Contributor

bbrowning commented Mar 13, 2025

@Mergifyio backport release-v0.7

@mergify
Copy link
Contributor

mergify bot commented Mar 13, 2025 •
edited
Loading

backport release-v0.7

✅ Backports have been created

Details

@bbrowning
Copy link
Contributor

bbrowning commented Mar 13, 2025

Yes, thanks for the nudge - we were just discussing this in our standup 😄

@mergify mergify bot mentioned this pull request Mar 13, 2025
Merged
bbrowning added a commit that referenced this pull request Mar 13, 2025
@bbrowning
Merge pull request #569 from instructlab/mergify/bp/release-v0.7/pr-549
6e1d140 
Add empty list check for element before table to fix "list index out of range" (backport #549)
@cfchase cfchase deleted the fix-table-chunker branch September 10, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RobotSail RobotSail RobotSail approved these changes

+3 more reviewers

@osilkin98 osilkin98 osilkin98 approved these changes

@aakankshaduggal aakankshaduggal aakankshaduggal approved these changes

@khaledsulayman khaledsulayman khaledsulayman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ci-failure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SDG fails on markdown table with preceding text

10 participants

@cfchase @khaledsulayman @Prithiviraj25 @bbrowning @jjasghar @RobotSail @ktdreyer @nathan-weinberg @osilkin98 @aakankshaduggal

Comments