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

Migrate mssql domain service to new module system #2547

Closed
wants to merge 9 commits into from

Conversation

DanielFran
Copy link
Member

Fix #2119

@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Merging #2547 (294e61b) into main (b454405) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 294e61b differs from pull request most recent head 2bb0e28. Consider uploading reports for the commit 2bb0e28 to get more accurate results

@@             Coverage Diff             @@
##                main     #2547   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
+ Complexity      2315      2301   -14     
===========================================
  Files            544       542    -2     
  Lines           9245      9235   -10     
  Branches         229       229           
===========================================
- Hits            9245      9235   -10     
Impacted Files Coverage Δ
...er/lite/generator/project/domain/DatabaseType.java 100.00% <100.00%> (ø)
.../database/mariadb/domain/MariaDBModuleFactory.java 100.00% <100.00%> (ø)
...ase/mssql/application/MsSQLApplicationService.java 100.00% <100.00%> (ø)
...boot/database/mssql/domain/MsSQLModuleFactory.java 100.00% <100.00%> (ø)
...frastructure/primary/MsSQLModuleConfiguration.java 100.00% <100.00%> (ø)
...boot/database/mysql/domain/MySQLModuleFactory.java 100.00% <100.00%> (ø)
...ase/postgresql/domain/PostgresqlModuleFactory.java 100.00% <100.00%> (ø)
...abase/sqlcommon/domain/SQLCommonModuleBuilder.java 100.00% <100.00%> (ø)
...ch/jhipster/lite/module/domain/JHipsterModule.java 100.00% <0.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af54a4b...2bb0e28. Read the comment docs.

@DanielFran
Copy link
Member Author

@pascalgrimaud Any clue why CI is failing?

@DanielFran DanielFran force-pushed the module_mssql branch 3 times, most recently from 2566d7d to b921e0e Compare July 14, 2022 14:14
@DanielFran
Copy link
Member Author

@pascalgrimaud Locally (in windows) I have mssqlapp that build fine, no issue downloading the docker image!?!
Any clue?

@DanielFran
Copy link
Member Author

@pascalgrimaud Found the problem, current configuration only works for 2017 mssql docker image version.
This will need to be fine-tuned, but we can work on this after module migration

@pascalgrimaud
Copy link
Member

Don't understand what is the problem in this migration

Because I used this feature for my current project for my customer, using same version here https://github.com/jhipster/jhipster-lite/blob/main/src/test/resources/generator/dependencies/Dockerfile#L9

And it works well

@DanielFran
Copy link
Member Author

DanielFran commented Jul 15, 2022

@pascalgrimaud I mean the current configuration for testcontainer.
The previous version use mssql v.2019 for prod and v.2017 for integration test with testcontainer (version "latest")
https://github.com/jhipster/jhipster-lite/blob/main/src/main/resources/generator/server/springboot/core/MssqlTestContainerExtension.java.mustache#L14

I wanted to use same image version for prod and test like currently in generator-jhipster
But using for example version 2019-CU16-GDR1-ubuntu-20.04 for testcontainer, it will fail with error: "manifest for mcr.microsoft.com/mssql/server:2019-cu16-gdr1-ubuntu-20.04 not found: manifest unknown: manifest tagged by "2019-cu16-gdr1-ubuntu-20.04" is not found"
https://github.com/jhipster/jhipster-lite/runs/7362845691?check_suite_focus=true#step:9:136

@pascalgrimaud
Copy link
Member

Oh I see, I'm fine to improve this after migration

@DanielFran DanielFran requested a review from pascalgrimaud July 15, 2022 23:11
@DanielFran DanielFran marked this pull request as ready for review July 15, 2022 23:11
@DanielFran
Copy link
Member Author

@pascalgrimaud Migration is now done

@DamnClin
Copy link
Collaborator

Can I help on this one?

@DanielFran
Copy link
Member Author

@pascalgrimaud @DamnClin I am on holidays since few days, and no access to any pc, so yes, you can rebase and merged it
Thanks

@pascalgrimaud
Copy link
Member

closing in favor of #2697

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.

Convert MSSQL to module
3 participants