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

finds classes in precompiled folder too #1152

Merged

Conversation

cbxp
Copy link
Contributor

@cbxp cbxp commented May 26, 2017

currently Play 1.5 cannot find any Entity classes in precompiled mode

@asolntsev asolntsev self-requested a review May 26, 2017 11:28
@asolntsev asolntsev added this to the 1.5.0 milestone May 26, 2017
@asolntsev asolntsev self-assigned this May 26, 2017
@asolntsev asolntsev merged commit 0c59058 into playframework:master May 26, 2017
@asolntsev asolntsev deleted the fix-hibernate5-in-precompiled-mode branch May 26, 2017 12:56
@flybyray
Copy link
Contributor

wait a minute, that patch makes no sense. in precompiled mode all classes are loaded before any plugin like the JPAPlugin is doing real work. if your entities are not there there is some other problem.
that fix may resolve somewhat the symptom but not the real problem. but just to late for today, i try to investigate that another time. at least it gave a hint, because those changes in applicationclassloader for the hibernate5 update is suspicious.

@asolntsev
Copy link
Contributor

asolntsev commented May 27, 2017 via email

@flybyray
Copy link
Contributor

flybyray commented May 29, 2017

just glad to have seen #1152
somehow it is good to describe errors in detail (stacktrace) on lighthouse or an github issue first.
then someone can make some reliable decission how to fix the real problem.
a classloader change just for upgrading a library is suspicious i did not recognized it by a fast review of #1114 .
i think the real fix could be something like #1155

@asolntsev
Copy link
Contributor

asolntsev commented May 29, 2017 via email

@flybyray
Copy link
Contributor

flybyray commented May 29, 2017

yes might be a good change in some (odd) cases. it was not needed before, maybe it would be interessting to have a trace logging in those conditional branches to see if it would effect any test case in play. maybe it should stay a feature which is turned off by default.
but in general separation of concerns should be applied to pull requests too. a library upgrade should result in minimum changes to existing source.

ps: those many pull request from my side lastly is a result of this https://play.lighthouseapp.com/projects/57987-play-framework/tickets/2067-fix-os-and-language-related-stuff#ticket-2067-5

ps: hmm a spam bot? https://play.lighthouseapp.com/projects/57987-play-framework/tickets?q=created%3A04-26-17&filter=

@flybyray
Copy link
Contributor

flybyray commented May 29, 2017

@asolntsev

It's really simpler and better solution for loading entity classes.

maybe not. as i stated in #1114 (review)
there is some commented todo in hibernate. the loaded_classes is for internal use . but it is for now the only way to get the behavior as it was with 4.x hibernate

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

Successfully merging this pull request may close these issues.

3 participants