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

Add support for RabbitMQ versions up to 3.7.7 #45

Merged

Conversation

pete-woods
Copy link
Contributor

@pete-woods pete-woods commented Sep 6, 2018

  • Add newest versions of RabbitMQ.
  • Switched default download source to GitHub as it spans more versions.
  • Add support for more digits to the version comparator.
  • Add bintray download source.
  • Fixes Missing support for newer RabbitMQ versions #44

@pete-woods
Copy link
Contributor Author

Struggling to see what's wrong with CI this second time. I fixed the initial checkstyle stuff. For what it's worth, the tests still pass reliably on my Mac.

@pete-woods
Copy link
Contributor Author

Okay, one of the CI issue looks like:

  • OSX: Need to use RabbitMQ 3.7.7 in the test to support homebrew's Erlang version 21.

@AlejandroRivera
Copy link
Owner

@pete-woods thanks for the PR! I've not looked into your code or the CI/CD situation closely enough to be able to help, but i'll do my best to do so ASAP (but probably won't be able to until early next week).

In the mean time, keep up fighting the good fight! 😄

@pete-woods
Copy link
Contributor Author

FYI, 3.7.3 is the newest version of 3.7.X I could get to run on OSX when specifying the new configuration format. The RabbitMQ guys seem to claim there is some packaging issue with Homebrew Erlang breaking it for newer releases.

@AlejandroRivera
Copy link
Owner

@pete-woods: i've looked more carefully at this PR and it looks great! The only thing i have my doubts about is the change in the repository implementation that prevents using a repo

I don't mind changing the default repository to be the GitHub impl (because the range of versions is more complete there), but preventing using the RabbitMQ website for newer versions feels too tightly coupled with distribution decisions that can change -- unless there's an official statement about it. Do you know if there's something like that that we can rely on to enforce it in code? If there isn't, I'd just remove this check and perhaps enhance the exception message indicating that not all versions can be found in all repos.

Regarding the failed builds:
a) Travis (which is used to build the project on Mac OS X) was configured to use JDK 7 (to avoid using the JDK 8 which was the default then. It seems that nowadays, Travis uses the JDK 10 as default and the previous mechanism for switching to JDK 7 is not present anymore. Because of this, I've updated it to use JDK 8. So... please merge master into your branch and see if it builds correctly in Travis now.

b) The build in AppVeyor (which is used to build the project in Windows) is failing because it can't verify the server is ready by waiting for the "completed with X plugins." message to appear after starting the rabbitmq-server executable. I've not had time to manually test things in a Windows box, so can you check if there's a need to adjust things in this platform?

PS. I'd have offered to help make some code changes in your PR directly, but you don't seem to have enabled this PR for maintainers to push code to your branch. See here for more info.

;

private static enum VersionSupport {
Copy link
Owner

Choose a reason for hiding this comment

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

fwiw, the static keyword is redundant

@@ -15,10 +15,17 @@
*/
public enum OfficialArtifactRepository implements ArtifactRepository {

RABBITMQ("http://www.rabbitmq.com/releases/rabbitmq-server/v%s.%s.%s/rabbitmq-server-%s-%s.%s"),
GITHUB("https://github.com/rabbitmq/rabbitmq-server/releases/download/rabbitmq_v%s_%s_%s/rabbitmq-server-%s-%s.%s"),
GITHUB("https://github.com/rabbitmq/rabbitmq-server/releases/download/%sv%s/rabbitmq-server-%s-%s.%s", VersionSupport.ANY),
Copy link
Owner

Choose a reason for hiding this comment

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

Oh, one more recommendation: keeping the enum order the same as before, and only adding new ones at the end, help ensure binary backwards compatibility in some cases.

@pete-woods
Copy link
Contributor Author

Will get that fixed too. Github seemed to be having issues for me yesterday, which prevented me from having a proper look at this.

With regards the repo thing, the official site hasn't seemed to offer link to its own repos for a while now, and points you to bintray or github: https://www.rabbitmq.com/download.html. It just looks to me like they stopped hosting their own downloads at 3.7 (http://www.rabbitmq.com/releases/rabbitmq-server/).

@pete-woods
Copy link
Contributor Author

15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server - dyld: lazy symbol binding failed: Symbol not found: _clock_gettime
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server -   Referenced from: /var/folders/my/m6ynh3bn6tq06h7xr3js0z7r0000gn/T/junit2466336418577859858/extracted/rabbitmq_server-3.7.3/erts-9.1/bin/../../erts-9.1/bin/beam.smp (which was built for Mac OS X 10.12)
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server -   Expected in: /usr/lib/libSystem.B.dylib
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server -
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server - dyld: Symbol not found: _clock_gettime
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server -   Referenced from: /var/folders/my/m6ynh3bn6tq06h7xr3js0z7r0000gn/T/junit2466336418577859858/extracted/rabbitmq_server-3.7.3/erts-9.1/bin/../../erts-9.1/bin/beam.smp (which was built for Mac OS X 10.12)
15:01:21.879WARN  Thread-4i.a.o.e.r.E.P.rabbitmq-server -   Expected in: /usr/lib/libSystem.B.dylib```

@pete-woods
Copy link
Contributor Author

Seems like Erlang is crashing on us (not installed correctly?) in the OSX build. Unfortunately I don't have a Windows machine to test the Windows build error with.

@pete-woods
Copy link
Contributor Author

Just checked the history for Homebrew's RabbitMQ formula (https://github.com/Homebrew/homebrew-core/blob/master/Formula/rabbitmq.rb). It switches away from rabbitmq downloads at 3.6.14. (i.e. just before 3.7)

@pete-woods
Copy link
Contributor Author

Hmm. AppVeyor build passed this time?? Didn't do anything to try and fix it..

@pete-woods
Copy link
Contributor Author

Also, pretty sure I've turned on the "Allow edits from maintainers" option now.

@pete-woods
Copy link
Contributor Author

pete-woods commented Sep 13, 2018

Just discovered this, RE the download host: http://www.rabbitmq.com/blog/2018/02/05/whats-new-in-rabbitmq-3-7/

Package Distribution Changes
Starting with 3.7.0, RabbitMQ packages (binary artifacts) are distributed using three services:

  • Bintray provides package downloads as well as a Debian and Yum (RPM) repositories
  • Package Cloud provides Debian and Yum repositories
  • GitHub releases include all release notes and provide a backup package download option

If you currently consume packages from rabbitmq.com, please switch to one of options above.

@coveralls
Copy link

coveralls commented Sep 15, 2018

Coverage Status

Coverage decreased (-5.1%) to 73.673% when pulling 76ee9a9 on pete-woods:44/newer-rabbitmq-versions into 003afe7 on AlejandroRivera:master.

…s depending on the version.

- Version object is responsible for parsing version string.
- Github repository enum is aware of it's own URL patterns.
- Giving more info on why RabbitMQ repository is now `@Deprecated`.
- RabbitMq repo logic enforces version check before generating URL.
@AlejandroRivera
Copy link
Owner

@pete-woods the issue with erlang crashing was due to the OSX image. I updated that in master and merged master in here. The issue with AppVeyor was just flaky test infrastructure. Some times, starting RabbitMQ in takes longer than the 20 seconds limit there's in the tests. Most times it's quite fast though. Just re-building the same commit takes care of the issue.

Check out the commits i introduced and lmk what you think. If you're fine with this and you're done with this PR, we can merge and release a new version asap.

@pete-woods
Copy link
Contributor Author

They seem like sensible evolutions to me! 👍

@pete-woods
Copy link
Contributor Author

pete-woods commented Sep 15, 2018

And yes, I'm done with this PR, from my perspective. I just need 3.7 support, so that my integration tests are using the same version as I'm using in production.

@AlejandroRivera
Copy link
Owner

Sounds good. I'll proceed to merge and release

@AlejandroRivera AlejandroRivera merged commit cea521c into AlejandroRivera:master Sep 15, 2018
@pete-woods pete-woods deleted the 44/newer-rabbitmq-versions branch September 15, 2018 17:12
@pete-woods
Copy link
Contributor Author

Thanks very much for your responsive reviews, and for getting this merged so quickly. It's much appreciated.

@AlejandroRivera
Copy link
Owner

Thank you for your contribution 😃

It's now released: https://github.com/AlejandroRivera/embedded-rabbitmq/releases/tag/v1.3.0
You can also find it in Sonatype: https://oss.sonatype.org/#nexus-search;quick~embedded-rabbitmq
It should be propagated to Maven central shortly.

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.

None yet

3 participants