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

upgrade version of crystal to 0.8.0 #1746

Merged
merged 3 commits into from Oct 19, 2015

Conversation

kbrock
Copy link
Contributor

@kbrock kbrock commented Oct 3, 2015

add postgres support too

@kbrock kbrock force-pushed the crystal branch 3 times, most recently from 829c058 to 8c617b2 Compare October 10, 2015 04:10
@kbrock
Copy link
Contributor Author

kbrock commented Oct 11, 2015

For some reason redis is working locally but not on these tests.

But it has postgres support - which is a better benchmark for comparing with other tests.

@kbrock
Copy link
Contributor Author

kbrock commented Oct 11, 2015

@zane-techempower was your thought that 1 program would serve all 3 database types?

Ideas why redis would be failing? It looks like it is not getting configured correctly. (I used the techempower script to populate my redis)

Thanks

@zane-techempower
Copy link
Contributor

@zane-techempower was your thought that 1 program would serve all 3 database types?

This is flexible, endpoints of a certain db can be separated by their route, e.g. /redis/json, /postgres/json. Or a whole separate server can be made. The way you have it currently is fine—your benchmark-config.json looks good, which is the important part as the toolset uses that to setup and test the endpoints.

Ideas why redis would be failing? It looks like it is not getting configured correctly. (I used the techempower script to populate my redis)

I believe you when you say that redis is working just fine locally, as we are aware of redis having issues on travis.ci. On travis, redis is set up in its own way, different to our other test environments, which is leading to erroneously failing tests. Fixing up the redis bug on travis is on my todo-list but I have not had time amongst my other responsibilities. Travis does great work for us, and this redis issue is a small unfortunate thing.

Two things can happen now:

  1. Someone fixes up the Redis on travis bug, removing those erroneously failing tests. Here's the issue, and the pr with some more discussion.
  2. Someone, likely @msmith-techempower will run your PR locally, confirm it works, and manually merge it. This is what we did to include redis-using frameworks for round 11. This will not happen until work for round 12 opens—which will happen shortly. We are currently on the verge of putting out round 11 data, and are not actively working to merge new contributions at the moment.

Thanks for the work! Crystal is a favorite of mine :)

@blee-techempower
Copy link
Contributor

Pass all localtests. Looks good.

 Verification Summary

| Test: moonshine
|       fortune     : PASS
|       plaintext   : PASS
|       db          : PASS
|       update      : PASS
|       json        : PASS
|       query       : PASS

blee-techempower added a commit that referenced this pull request Oct 19, 2015
upgrade version of crystal to 0.8.0
@blee-techempower blee-techempower merged commit 88cb45a into TechEmpower:master Oct 19, 2015
@kbrock kbrock deleted the crystal branch October 22, 2015 04:27
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.

None yet

3 participants