TAMULib / SAGE

Search Aggregation Engine
MIT License
6 stars 3 forks source link

Issue 506: Make database pre-populate specific to on 'create'. #507

Closed kaladay closed 1 year ago

kaladay commented 1 year ago

Description

Use spring.sql.init.platform to designate the file to use. It seems that we have been settings this for no apparent reason in the past. Change the platform options to create, update, and none.

I chose update if we ever need to have a data-update.sql that gets run on every update. (For example, a hypothetical database property that has 'last_started' database table and property that gets set to the date on every system start.)

The documentation specifically states the following:

This allows you to switch to database-specific scripts if necessary. For example, you might choose to set it to the vendor name of the database (hsqldb, h2, oracle, mysql, postgresql, and so on).

This tells me that the previous choice of using something like postgresql or h2 is superficial given the lack of utilization. If we want databae-specific versions, we could have additional platforms as needed. All we need to do to disable this is to set this to something like none.

This change has a downside of requiring yet another step in the deployment process.

All systems that use environment variables to designate properties should now be sure to set spring.sql.init.platform to update.

see: https://docs.spring.io/spring-boot/docs/current/reference/html/howto.html#howto.data-initialization.using-basic-sql-scripts see: https://docs.spring.io/spring-boot/docs/2.1.x/reference/html/howto-database-initialization.html#howto-initialize-a-database-using-spring-jdbc

Fixes #506

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Test Configuration: Postgresql and H2

Checklist:

ghost commented 1 year ago

We have been setting platform to provide ability to afford ORM to switch database vendors as described as a possible use case in the documentation.

What is this trying to solve?

Why not just create another property file for development and not commit it to handle development?

application-dev.yml

spring.sql.init.mode: alwayws => none
spring.jpa.hibernate.ddl-auto: create-drop => update

mvn clean spring-boot:run -Dspring-boot.run.profiles=dev

coveralls commented 1 year ago

Coverage Status

Coverage: 45.252%. Remained the same when pulling d170dea53df3d4bdb15136bdc4e6302a2d97bfe3 on 506-duplicate_key-metadatum into 6119eb91d2f727337ef06bc265dcf7542ab7b612 on staging.

kaladay commented 1 year ago

What is this trying to solve?

The issue described in the subject/title, the descrion, and referenced issue number.

Why not just create another property file for development and not commit it to handle development?

Would need application-dev-create.yml, application-dev-update.yml, and maybe application-dev-none.yml. Production needs to run in production mode, so we would also need:

Then all of the configuration options would need to be duplicated.

The spring.sql.init.mode is a good candidate for solving this, but we are setting it to update because we do want database initialization done. Aren't there things that we do want to update?

Interestingly, we already have non-H2 database SQL in the data.sql. The query:

ALTER SEQUENCE IF EXISTS internal_metadata_id_seq RESTART WITH 42;

is postgresql-specific in that H2 does not build sequences like that. There is no internal_metadata_id_seq so that functionality does nothing. The IF EXISTS is the only thing keeping that from crashing. Perhaps H2 is broken then?

We might need a data-postgresql.sql with that line in it then. But then there is a question of order of operations. Which is run first data.sql or data-postgres.sql? I want to assume the former is run first. Or is it a one or the other situation?

This raises another question. The data.sql runs:

... M WHERE NOT EXISTS(SELECT * FROM INTERNAL_METADATA);

Could it be the desired behavior that on every run we must assure those rows exist? That is, if someone deleting any of those then on the next run with ddl-auto set to either create/create-update or update the script will add them back?

ghost commented 1 year ago

By issue, I mean a problem or something that is not working or presenting efficiency loss. I agree it is a topic worth discussing but not convinced changes in code (or versioned configuration) and data import file names are yet required. Possibly using environment variables will be easy for your development. I see this as; do we want to proliferate configuration files or configuration values or use what is available and mutate for current use case?

kaladay commented 1 year ago

Alternative PR that treats this as a documentation problem and a configuration issue.

kaladay commented 1 year ago

Resolved instead by #508.