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

FlywayConfiguration parameters configuration #450

Merged
merged 7 commits into from
Sep 20, 2023

Conversation

driverpt
Copy link
Contributor

@driverpt driverpt commented Sep 8, 2023

Fixes #442

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2023

CLA assistant check
All committers have signed the CLA.

patch.diff Outdated Show resolved Hide resolved
@@ -53,6 +57,7 @@ public class FlywayConfigurationProperties implements Toggleable {
private String url;
private String user;
private String password;
private Map<String, String> properties = new Hashtable<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why HashTable? This will be a singleton which will be used only on startup. It does not need to support synchronisation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't want to use HashMap, I think we should use https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the Base Java version for Micronaut Flyway ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17

Copy link
Contributor Author

@driverpt driverpt Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I switch this to Map.of() instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed this to HashMap 910c8b2

@@ -182,4 +187,24 @@ public boolean hasAlternativeDatabaseConfiguration() {
public FluentConfiguration getFluentConfiguration() {
return fluentConfiguration;
}

/**
* Sets extra properties to be applied on configuration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The java doc from Flyway FluentConfiguration it says Property names are documented in the flyway maven plugin. @driverpt do you have link to those properties that we could add to the javadoc here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is what you're looking for: https://documentation.red-gate.com/fd/parameters-184127474.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have improved the javadoc c475adc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have also added an example in the docs 1a56988

@sdelamo sdelamo added the type: enhancement New feature or request label Sep 19, 2023
@sdelamo sdelamo changed the title Fix #442 FlywayConfiguration parameters configuration Sep 19, 2023
@sdelamo sdelamo merged commit 1a80a6b into micronaut-projects:master Sep 20, 2023
3 checks passed
@driverpt driverpt deleted the fix-442 branch September 20, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Allow Custom Properties to be set
4 participants