Skip to content

Conversation

@radixhound
Copy link

@radixhound radixhound commented Oct 22, 2016

In order to get the ruby client working with my Rails 5 based project, I had to remove the json dependency constraints as well as activesupport and rest_client constraints since I was locked to newer versions of these.

In theory this client should be designed to work with whatever json package is used by the project that it gets included in, rather than bringing in it's own json dependency.

  • specs pass with this change except for one spec that seems to depend on launching an actual server to test against.
  • client works with my Rails 5 project
  • maybe would be a good idea to update specs to test against different json gems?

NB: the specs call the to_json method in Enum. My project calls the as_json method in Enum so the as_json case is not currently covered in the specs.

Let me know if this seems like a good direction. I couldn't tell if this was the final version of the ruby client or if there's still a new version in the works. Cheers!

- Locking the json to an old version seems uneccessary
- Forcing multi_json and json_pure monkeypatching broke our test suite
- removed the json_pure and so far so good
- in the specs it uses some json gem to render json which calls to_json
  on the object
- in my app it uses an implementation that calls as_json on the object
- to get the specs passing, to_json had to include quotes
- to get my app to work as_json should not include quotes
@jamesdbloom
Copy link
Collaborator

jamesdbloom commented Oct 27, 2016

This PR can't be merged until the build for this PR is fixed, thanks.

@radixhound
Copy link
Author

@jamesdbloom I spent a little while with this and was unable to make headway. It seems like there is some configuration for the Snap CI build that I don't have access to? It needs to be updated to install a new ruby version on the CI runner. Things I tried:

  • Update travis.yml to bump the ruby version
  • Ran the tests locally (there was one failure but unrelated to the ruby client)
  • Ran the tests on my own Snap CI - (this didn't run ruby tests at all since it didn't auto-detect it)

If you can give me an idea how I can work with that Snap CI build, I'm happy to keep working on it.

@jamesdbloom
Copy link
Collaborator

jamesdbloom commented Nov 4, 2016 via email

@radixhound
Copy link
Author

Hmm thinking this through from the current point:

  • The ruby upgrade is needed because the latest version of active_support needs it
  • ActiveSupport needs it because removing the ~> 4.1 restriction causes it to install 5.0.0.1
  • active_support in the gemspec needs to be updated from ~> 4.1 because that will lock it to the 4.x range and keep it from working for Rails 5 projects.

Perhaps we could update tests to explicitly use ActiveSupport 4.x then the tests will run against that version and the ruby upgrade won't be necessary?

Or perhaps make a separate Rails 5 compatible branch?

Or maybe the client code could be updated to not depend on ActiveSupport?

Generally, I think the direction of removing the json restrictions will only help to make it more compatible. The tricky part there is how to test with the various possible json gem options out there and make sure they still work? Probably best to start by setting up the test environment to have certain gems installed and at least cover the use cases it had before this change to ensure that if people had it working in a given environment, it should continue working there?

That's a lot of question marks... Sorry for generating more questions than answers. I'm happy to tackle one or more of these solutions though once I have an idea what you'd like to see.

@ovidiubite
Copy link

@jamesdbloom @radixhound Hey. guys. I read all the comments from this issue and it seems that for a good period of time nothing happened. Is this PR abandoned? I have the same issue with integrating the gem with a Rails 5 application due to the activesupport dependencies. I think it's a big deal to not be able to use it with Rails 5 and we should definitely do something to fix it.

@radixhound
Copy link
Author

@ovidiubite This branch is working very well for us on our Rails 5 project. I'd love to help get this update merged in but my hands are tied without some guidance.

@ovidiubite
Copy link

ovidiubite commented Mar 29, 2017

@radixhound I think I'm also gonna use your branch until we manage to merge the update. Another potential issue for this repo is the end of life for Snap Ci https://blog.snap-ci.com/blog/2017/02/06/2017-02-06-snap-announcement/ but this is another topic.

@jamesdbloom
Copy link
Collaborator

Unfortunately this PR can't be merged as its failing the build.

@jamesdbloom
Copy link
Collaborator

On the topic of Snap CI no longer supporting builds, I'm looking at move back to Travis or drone.io to resolve that.

@jamesdbloom
Copy link
Collaborator

the ruby client has moved to https://github.com/jamesdbloom/mockserver-client-ruby please raised any PRs for this client on that repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants