jasmineRepo / JAS-mine-core

JAS-mine maintains and develops the JAS simulation platform, a discrete-event tool-kit for agent-based and dynamic microsimulation modelling. This repository contains the core libraries. See www.jas-mine.net for more details.
2 stars 5 forks source link

Missing imports in DatabaseUtils in v5.0 #23

Closed pbronka closed 2 years ago

pbronka commented 2 years ago

Hi @vkhodygo,

@justin-ven and I tried to use the newest version of JAS-mine core to do some testing but ran into issues with DatabaseUtils. Would you be able to have a look please?

Issue: DatabaseUtils from MRC-CSO-SPHSU/JAS-mine-core/tree/dev/5.0.0 uses SchemaExport and SchemaUpdate in the code (inputSchemaUpdateEntityManger() and getOutEntityManger(String persistenceUnitName) methods) but the imports (import org.hibernate.tool.hbm2ddl.SchemaExport; import org.hibernate.tool.hbm2ddl.SchemaUpdate) are failing.

vkhodygo commented 2 years ago

Hi,

I should warn you it's not in the state for any testing at the moment. You can try, but pick the correct version of hibernate. The database part was extremely outdated and required some serious upgrades. I managed to bump the hibernate version to 5.8.1 or something, but I'm not sure if the code works correctly. However, 6.0.1 and later make all this code completely invalid as the API doesn't support this anymore. This part has to be rewritten from scratch I'm afraid.

pbronka commented 2 years ago

Thanks for looking into this. Some of the features included in the 6.1.0 version of Hibernate (which has been released in the last two weeks) would be really nice to have and it probably makes sense to go with the newest version instead of making the code work with 5.8.1 and then starting over when we try to move to the 6.1.0+? Were you planning to rewrite this part of code soon? Perhaps this is something that we could prioritise?

vkhodygo commented 2 years ago

Yes, that was my plan. Unfortunately, I had to work on some other things and it took all my time. I'm on leave next week and I'm going to get back to the database section right after that.

P.S. What are those features exactly?

vkhodygo commented 2 years ago

Hi @pbronka

Apologies for the delay, let's get back to our business. What features exactly do you need? It would be great to see two lists: for required and optional features, respectively.

pbronka commented 2 years ago

Hi @vkhodygo,

  1. Are you planning to update both JAS-mine core and LABSim so that it works with the new version of JAS-mine core?
  2. I think the main thing here would be the update of JAS-mine core to the most current JPA library, which will include a full migration from javax to Jakarta. This will further require updating of other dependencies and code of JAS-mine core and associated integrations with LABSim.

Edit: The reason for the switch to the most current library is that the switch over from javax to Jakarta represents a fairly significant mile-stone in JPA, which will be important in managing the library looking forward. For example, database queries via the entity manager are more simple with the update (e.g. referencing object dependencies no longer requires odd boiler-plate code, with reference to query “hints”), and differences of this type are only likely to become greater with time.

vkhodygo commented 2 years ago

@pbronka

Are you planning to update both JAS-mine core and LABSim

Both, but JAS-mine comes first as a more basic package.

the most current JPA library

That's my plan, however, I'm not a database expert, so I need you feedback on this one.

justin-ven commented 2 years ago

On the database issue, suggest that all we need is for JAS-Mine to be updated to the most recent JPA library, and are not after additional functionality from the JAS-Mine library in this respect. The reason for the update is that this affects the types of database calls that we can make from labsim, as the JPA library used by labsim needs to be the same as JAS-Mine (we have experimented with using a different one in labsim, with no success).

Justin.

From: Vladimir @.> Sent: Thursday, 7 July 2022 5:06 PM To: jasmineRepo/JAS-mine-core @.> Cc: justin-ven @.>; Mention @.> Subject: Re: [jasmineRepo/JAS-mine-core] Missing imports in DatabaseUtils in v5.0 (Issue #23)

@pbronkahttps://github.com/pbronka

Are you planning to update both JAS-mine core and LABSim

Both, but JAS-mine comes first as a more basic package.

the most current JPA library

That's my plan, however, I'm not a database expert, so I need you feedback on this one.

— Reply to this email directly, view it on GitHubhttps://github.com/jasmineRepo/JAS-mine-core/issues/23#issuecomment-1177759634, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKJL7NFIAY5JQH2WFLJT46DVS3W3XANCNFSM5ZXN6QFA. You are receiving this because you were mentioned.Message ID: @.***>

pbronka commented 2 years ago

@pbronka

Are you planning to update both JAS-mine core and LABSim

Both, but JAS-mine comes first as a more basic package.

I appreciate that the version 5.0 of JAS-mine core is under development, but when we tried to use it with LABSim, with many updated libraries, we encountered multiple issues that would have to be fixed. Therefore, one suggestion would be to start with updating only the JPA libraries (as Justin suggests above) and integrating that update with LABSim, before moving to a new problem. In that way, not only changes can be tested, but development of models can continue with updated libraries.

the most current JPA library

That's my plan, however, I'm not a database expert, so I need you feedback on this one.

I would suggest the following: taking the last stable version of JAS-mine core which works with LABSim (4.0.11), updating Hibernate:

`

org.hibernate
        <artifactId>hibernate-entitymanager</artifactId>
        <version>4.1.10.Final</version>
    </dependency>`

to most current version 6.1.1. and `

org.hibernate.javax.persistence
        <artifactId>hibernate-jpa-2.1-api</artifactId>
        <version>1.0.0.Final</version>
    </dependency>`

to Jakarta persistence 3.1 and updating any related dependencies and resolving issues that will inevitably come up. Then integrating the new version with LABSim, before moving to a new issue. Does that sound ok?

pbronka commented 2 years ago

Hi @vkhodygo ,

Just wanted to see if you might have any thought on this?

Thanks

vkhodygo commented 2 years ago

Hi @pbronka

I have some good news for you. I went through the whole codebase and got everything (sort of) fixed. Now labsim can run with the most recent version of jasmine. It throws some errors, but they don't look critical to me. The only serious issue here is related to database schemas. I thing I can create new ones with the most recent Jakarta API, but I still have no idea how to update already existing ones.

Please, let me know what you'd like to see next.

pbronka commented 2 years ago

Hi @vkhodygo

The new version of JAS-mine core loads a jamjam 0.0.1 library as dependency in the pom.xml file, but this does not seem to be available from Maven's central repository. I'm not sure what org.uog is. I tried to clone jamjam from your GitHub but failed; furthermore it was unclear to me which commit would be a 0.0.1 version. Could you point me in the right direction? Thanks!

pbronka commented 2 years ago

Hi @vkhodygo

(I have resolved the issue above by pointing JAS-mine core to a particular commit as dependency using jitpack.io to load it.

  <dependency>
      <groupId>com.github.MRC-CSO-SPHSU</groupId>
      <artifactId>jamjam</artifactId>
      <version>38a0051</version>
  </dependency>

I also had to replace the loading of the maven-assembly-plugin with a version from Maven's repository:

      <!-- https://mvnrepository.com/artifact/org.apache.maven.plugins/maven-assembly-plugin -->
      <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-assembly-plugin</artifactId>
          <version>3.3.0</version>
      </plugin>

However, I am now getting exactly the same problem as in the very first message in this issue:

Issue: DatabaseUtils from MRC-CSO-SPHSU/JAS-mine-core/tree/dev/5.0.0 uses SchemaExport and SchemaUpdate in the code (inputSchemaUpdateEntityManger() and getOutEntityManger(String persistenceUnitName) methods) but the imports (import org.hibernate.tool.hbm2ddl.SchemaExport; import org.hibernate.tool.hbm2ddl.SchemaUpdate) are failing.

Given this comment

I have some good news for you. I went through the whole codebase and got everything (sort of) fixed. Now labsim can run with the most recent version of jasmine. It throws some errors, but they don't look critical to me.

could you please provide some further instructions how to run the new version of JAS-mine core with LABSim?

vkhodygo commented 2 years ago

@pbronka

Could you point me in the right direction?

Just clone the repo and build/install it as it is. I didn't have enough time to publish a proper release so I simply use the latest version.

I have resolved the issue above by pointing JAS-mine core to a particular commit

That's a nice feature I didn't know about! Thanks.

I also had to replace the loading of the maven-assembly-plugin

I don't recall any issues with accessing this plugin, that's odd. Still, it's good you got it fixed.

could you please provide some further instructions

That's the thing I wanted to discuss that badly. I think I spent at least a week reading various sources and trying different approaches, but I still have one Jakarta-related issue. There is no hbm2ddl module anymore, and the release notes at the time of complete removal say there is no simple way to migrate old schema code to the new API or something. The recommendation is to ask on their forum, can't find that document though.

Nevertheless, I've updated the method to create new schemas and it seems to work. I get an error while connecting to a DB, but that's probably because I don't have a proper environment to test that. However, I could not find any information related to schema updates. There is simply no such functionality, it's either this or my comprehension skills failed me.

I removed this part of the code to allow jasmine to build, but I didn't want to go any further than that without a proper discussion.

pbronka commented 2 years ago

Thanks for the clarification. My understanding of this part of the code is very limited, unfortunately.

From what I understand, hbm2ddl was automatically generating and updating the schema based on entities in the simulation, but we will now have to find a replacement for it? (Would one option be to write the schema manually? But then we would have to update it every time something changes in the simulation?)

Would a call (including Justin as he has more experience with the databases) be helpful to discuss how to proceed with this?

vkhodygo commented 2 years ago

hbm2ddl was automatically generating and updating the schema based on entities in the simulation, but we will now have to find a replacement for it?

That's how I see it too. We can find a workaround for that, but I have a felling it might look like a fix.

Yeah, I think having a call is a nice idea. Feel free to arrange a meeting any time you find convenient.

pbronka commented 2 years ago

Just to keep track of things, I'm closing this issue as @justin-ven updated dependencies and database code and it works as required now. These changes are on the following branch https://github.com/jasmineRepo/JAS-mine-core/tree/dev/4.0.12 and I will merge to master and do a release soon.

vkhodygo commented 1 year ago

@pbronka @justin-ven Apologies for the delayed reply, I was knee-deep in some issues at the time.

This is not how it's supposed to work. I've been working on this part as well as some others too, and now we have two versions of the code that are not compatible and will require some meticulous and manual intervention to merge which is what we don't need. In addition to that, we just did essentially the same thing twice.

pbronka commented 1 year ago

I agree that this is not how it's supposed to work, which is why we suggested you start with a working version and help us update the libraries and database-related code as a priority and release a new working version. But this was back in June / July and we've been waiting months on what turned out to be a day of work. It is unfortunate that we had to work on the same issue but we were not able to wait any longer.

Going forward we would strongly suggest that changes are small, well-tested, documented and explained, and result in a fully working version of the libraries that can be released. That way, everyone can work on some aspect of the libraries and we can ensure there is always an up-to-date working version available.

If you would like to do that, please contribute to this issue with the GUI and we can release a new version that fixes it.

pbronka commented 1 year ago

I agree that this is not how it's supposed to work, which is why we suggested you start with a working version and help us update the libraries and database-related code as a priority and release a new working version. But this was back in June / July and we've been waiting months on what turned out to be a day of work. It is unfortunate that we had to work on the same issue but we were not able to wait any longer.

Going forward we would strongly suggest that changes are small, well-tested, documented and explained, and result in a fully working version of the libraries that can be released. That way, everyone can work on some aspect of the libraries and we can ensure there is always an up-to-date working version available.

If you would like to do that, please contribute to this issuehttps://github.com/jasmineRepo/JAS-mine-gui/issues/7 with the GUI and we can release a new version that fixes it.

Regards, Patryk

From: Vladimir @.> Sent: 20 September 2022 18:34 To: jasmineRepo/JAS-mine-core @.> Cc: Bronka, Patryk @.>; Mention @.> Subject: Re: [jasmineRepo/JAS-mine-core] Missing imports in DatabaseUtils in v5.0 (Issue #23)

CAUTION: This email was sent from outside the University of Essex. Please do not click any links or open any attachments unless you recognise and trust the sender. If you are unsure whether the content of the email is safe or have any other queries, please contact the IT Helpdesk.

@pbronkahttps://github.com/pbronka @justin-venhttps://github.com/justin-ven Apologies for the delayed reply, I was knee-deep in some issues at the time.

This is not how it's supposed to work. I've been working on this part as well as some others too, and now we have two versions of the code that are not compatible and will require some meticulous and manual intervention to merge which is what we don't need. In addition to that, we just did essentially the same thing twice.

— Reply to this email directly, view it on GitHubhttps://github.com/jasmineRepo/JAS-mine-core/issues/23#issuecomment-1252685521, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ANPWCGYJIQY5CULSPSBQ2LTV7HYQLANCNFSM5ZXN6QFA. You are receiving this because you were mentioned.Message ID: @.**@.>>