implement first version (basic HTTP actions support, basic auth) #25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rursprung
commented
May 14, 2024
src/main/java/liquibase/ext/opensearch/changelog/OpenSearchHistoryService.java
Outdated
Show resolved
Hide resolved
filipelautert
force-pushed
the
initial-featureset
branch
from
b6850e2
to
9690c01
Compare
This was referenced May 15, 2024
rursprung
commented
May 15, 2024
There was a problem hiding this comment.
@filipelautert & @reta (and anyone else reviewing this! i added filipe as the stand-in mention for "liquibase team"): sorry for the spam, but i've added some comments to highlight specific things to you. please nevertheless review everything - i mainly highlighted the areas where i'm unsure (+ i found some things which i hadn't found before when i implemented this 😄)
src/main/java/liquibase/ext/opensearch/change/HttpRequestChange.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/ext/opensearch/changelog/OpenSearchHistoryService.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/ext/opensearch/changelog/OpenSearchHistoryService.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/ext/opensearch/changelog/OpenSearchHistoryService.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/nosql/database/AbstractNoSqlDatabase.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/nosql/snapshot/NoSqlSnapshotGenerator.java
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1 @@ | |||
| liquibase.ext.opensearch.database.OpenSearchLiquibaseDatabase | |||
There was a problem hiding this comment.
missing newline at EOF
filipelautert
requested changes
May 22, 2024
reta
reviewed
May 24, 2024
pom.xml
Outdated
| <dependency> | ||
| <groupId>org.opensearch.client</groupId> | ||
| <artifactId>opensearch-java</artifactId> | ||
| <version>2.10.2</version> |
There was a problem hiding this comment.
Couple of bugfixes included:
Suggested change
| <version>2.10.2</version> | |
| <version>2.10.3</version> |
There was a problem hiding this comment.
We could also exclude opensearch-rest-client since we use AHC 5 here (https://github.com/liquibase/liquibase-opensearch/pull/25/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R91)
<exclusions>
<exclusion>
<groupId>org.opensearch.client</groupId>
<artifactId>opensearch-rest-client</artifactId>
</exclusion>
</exclusions>
There was a problem hiding this comment.
the exclusion is blocked by opensearch-project/opensearch-java#1002 (though a workaround would be possible it's better to wait for the proper fix)
rursprung
force-pushed
the
initial-featureset
branch
from
9690c01
to
0a9f67e
Compare
rursprung
force-pushed
the
initial-featureset
branch
from
0a9f67e
to
6c0d9db
Compare
rursprung
force-pushed
the
initial-featureset
branch
from
6c0d9db
to
ce42aa9
Compare
rursprung
force-pushed
the
initial-featureset
branch
from
ce42aa9
to
8ac7cff
Compare
|
for the tests to pass this needs a new |
filipelautert
approved these changes
Jun 5, 2024
filipelautert
approved these changes
Jun 5, 2024
reta
reviewed
Jun 5, 2024
|
rursprung
commented
Jun 27, 2024
rursprung
force-pushed
the
initial-featureset
branch
from
54794a6
to
b7d359b
Compare
filipelautert
approved these changes
Sep 10, 2024
src/main/java/liquibase/nosql/database/AbstractNoSqlDatabase.java
Outdated
Show resolved
Hide resolved
src/main/java/liquibase/nosql/database/AbstractNoSqlDatabase.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| if (!hasDatabaseChangeLogTable()) { | ||
| getLogger().info("Create Database Change Log Collection"); |
filipelautert
force-pushed
the
initial-featureset
branch
from
b7d359b
to
61342ed
Compare
filipelautert
temporarily deployed
to
external
GitHub Actions
Inactive
note that the pom so far claimed that the license was the Liquibase EULA, but this repository - and the extension - is licensed as Apache 2.0.
this folder is generated by maven when running builds.
this already allows connecting to OpenSearch using basic authentication. note: the `liquibase.nosql` package has been copied from [`liquibase-mongodb`] and adapted where needed (it is not 100% generic). no authorship is claimed for this content! [`liquibase-mongodb`]: https://github.com/liquibase/liquibase-mongodb
this also comes with various supporting infrastructure, e.g. the executor. this does not yet allow executing any changesets, but it allows liquibase to run with an empty changeset which will create the `changelog` and also manage the lock (which ensures that only one liquibase operation is running at a time). note: the `liquibase.nosql` package has been copied from [`liquibase-mongodb`] and adapted where needed (it is not 100% generic). no authorship is claimed for this content! it's important that all document operations being performed on the changelog & lock indices are done with `refresh=wait_for` to ensure that a subsequent read will see the correct data (otherwise a changeset might be marked as run but a subsequent run will not see this and try to run it again). [`liquibase-mongodb`]: https://github.com/liquibase/liquibase-mongodb
this adds a new change type which allows generic HTTP requests. while this isn't great to support working over multiple OpenSearch major releases (in case the REST API had breaking changes) it already offers an easy way to trigger any possible action on OpenSearch. latter versions might add additional change types for specific operations on OpenSearch which can then also abstract away from the exact API version (if possible).
this allows executing the [`clear-checksums`] liquibase action. [`clear-checksums`]: https://docs.liquibase.com/commands/utility/clear-checksums.html
this provides the necessary XSD. note that the XSD has to be published at http://www.liquibase.org/xml/ns/opensearch/liquibase-opensearch-1.0.xsd for the reference to be valid for consumers. this seems to have been done for other plugins (e.g. mongodb) but is - presumably? - a manual process.
rursprung
force-pushed
the
initial-featureset
branch
from
61342ed
to
d7b8d16
Compare
|
|
🥳 |
rursprung
added a commit
that referenced
this pull request
Sep 19, 2024
this was a last-minute change before merging PR #25 and we didn't spot this: all tests are failing due to this as this is being called by the constructor of `ChangeLogParameters`. instead of returning something artifical (like `//`) which doesn't apply here we'll just return an empty string. this is needed until liquibase adds first-class support for NoSQL DBs and removes any SQL-specific features (requires liquibase/liquibase#4236).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
please see the individual commit messages for further details. this should give you a working first version of
liquibase-opensearchwhich can be used against OpenSearch as long as it is using basic authentication.please carefully review any open TODOs (none of which is a blocker, but some might suggest low-hanging fruits in
liquibase-corefor better no-SQL support) as well as the portedliquibase.nosqlpackage (taken from liquibase-mongodb; which isn't really generic!).closes #17