Skip to content

Conversation

@ghanko
Copy link
Contributor

@ghanko ghanko commented Feb 4, 2022

No description provided.

@tez-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 12m 44s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
_ Patch Compile Tests _
+1 💚 jshint 0m 5s There were no new jshint issues.
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
_ Other Tests _
+1 💚 asflicense 0m 58s The patch does not generate ASF License warnings.
14m 27s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-183/1/artifact/out/Dockerfile
GITHUB PR #183
Optional Tests dupname asflicense jshint
uname Linux 9a23921f5e15 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/tez.sh
git revision master / 6d7ef20
Max. process+thread count 51 (vs. ulimit of 5500)
modules C: tez-ui U: tez-ui
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-183/1/console
versions git=2.25.1 maven=3.6.3 jshint=2.12.0
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@abstractdog
Copy link
Contributor

@ghanko , @csjuhasz-c: fix makes sense to me, however, it's not clear how could have paging worked in tez ui without this patch until now? anyway, have you been able to reproduce this and validate the fix?

@ghanko
Copy link
Contributor Author

ghanko commented Feb 6, 2022

@ghanko , @csjuhasz-c: fix makes sense to me, however, it's not clear how could have paging worked in tez ui without this patch until now? anyway, have you been able to reproduce this and validate the fix?

How it used to work is not clear to me either, but now we could reproduce the issue by feeding it with mock data as described in the ticket. The result was exactly the same and after some debugging we found that the root cause is the [].concat() call as described here: emberjs/ember.js#9843 (comment)

The suggested fix of calling toArray() on the array being added didn't work because it wrapped the content elements to some other objects (probably a different version), but using the content property did fix the issue locally.

@abstractdog
Copy link
Contributor

@ghanko , @csjuhasz-c: fix makes sense to me, however, it's not clear how could have paging worked in tez ui without this patch until now? anyway, have you been able to reproduce this and validate the fix?

How it used to work is not clear to me either, but now we could reproduce the issue by feeding it with mock data as described in the ticket. The result was exactly the same and after some debugging we found that the root cause is the [].concat() call as described here: emberjs/ember.js#9843 (comment)

The suggested fix of calling toArray() on the array being added didn't work because it wrapped the content elements to some other objects (probably a different version), but using the content property did fix the issue locally.

thanks for clarifying @ghanko
+1

@abstractdog abstractdog self-requested a review February 6, 2022 17:45
@abstractdog abstractdog merged commit 9f4cb31 into apache:master Feb 14, 2022
asfgit pushed a commit that referenced this pull request Feb 14, 2022
…aba Juhasz, Gergely Hanko reviewed by Laszlo Bodor)
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