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

IGNITE-12358 Migrate ZeroMQ module to ignite-extensions #5

Closed
wants to merge 3 commits into from

Conversation

samaitra
Copy link
Member

No description provided.

Comment on lines 39 to 41
<ignite-core.version>2.9.0-SNAPSHOT</ignite-core.version>
<ignite-log4j.version>2.9.0-SNAPSHOT</ignite-log4j.version>
<ignite-spring.version>2.9.0-SNAPSHOT</ignite-spring.version>

Choose a reason for hiding this comment

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

it should be a single ignite.version, we do not need to have a separate version for each jar.
Furthermore, why do we have it here? I think it should be defined in parent pom, so all extensions will be built and release for the same ignite version. However, if we want to treat all extensions as absolutely independent unites and each one can support different ignite versions, then why we put all extensions under a single umbrella project?

Copy link
Member Author

@samaitra samaitra Mar 8, 2020

Choose a reason for hiding this comment

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

@Mikhail1988 I have updated the PR to have single ignite.version.

The plan is to only release each extension if they need to be updated based on latest ignite.version. This will help us reduce number of extension releases every ignite version release. We do need to certify and test that existing extension does not break with any new changes.

Comment on lines 19 to 27
<dependencies>
...
<dependency>
<groupId>org.apache.ignite</groupId>
<artifactId>ignite-zeromq</artifactId>
<version>${ignite.version}</version>
</dependency>
...
</dependencies>

Choose a reason for hiding this comment

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

it's not true anymore and needs to be fixed.
but I like idea to match extension version and ignite version, so let's replace 1.0.0-SNAPSHOT with ignite version 2.9.0-SNAPSHOT.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the docs


# node01 signed with trust-one, node02 and node03 by trust-two, node02old is expired
# trust-both contains both CAs
ssl.keystore.node01.path=@{IGNITE_HOME}/modules/clients/src/test/keystore/ca/node01.jks
Copy link

Choose a reason for hiding this comment

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

Do we need all of these properties here? These files are not reachable from ignite-extensions

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I had initially put it under ignite-extensions path but the tests were failing. So, I had put it in a locally resolvable path.

@@ -44,6 +44,7 @@
<modules>
<module>modules/flink-ext</module>
<module>modules/pub-sub</module>
<module>modules/zeromq-ext</module>
Copy link

Choose a reason for hiding this comment

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

Do we really need to add -ext to all of those modules' names?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had put ext to avoid conflict with existing released version. As we are planning to release from 1.0.0 version.

@asfgit asfgit closed this in fce59f2 Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants