correct versions order and show only indexed versions by sonalkr132 · Pull Request #1284 · rubygems/rubygems.org

sonalkr132 · 2016-05-22T13:24:53Z

Looks like I have found more tests which fail only on travis 😓

preserve order test on bunder-api.
Indexed test on bundler-api.

homu · 2016-05-22T15:24:22Z

☔ The latest upstream changes (presumably #1282) made this pull request unmergeable. Please resolve the merge conflicts.

sonalkr132 · 2016-05-23T03:51:44Z

Although test have passed and they are correct, we still need to figure out why they were failing previously.
I had to use different gem names because database is not cleaning between test runs (only on travis). Objects created in unit tests were breaking functional test just cause they used the same name (travis link).

It is exactly what was making #1281 fail (travis build link).

This is all I know so far and I don't know where to go with this.

homu · 2016-05-24T16:22:47Z

☔ The latest upstream changes (presumably #1286) made this pull request unmergeable. Please resolve the merge conflicts.

dwradcliffe · 2016-05-25T03:08:58Z

The code looks good. I'm not sure about the Travis failures, and we should figure that out, but I think we should proceed with this anyway.

sonalkr132 · 2016-05-25T04:05:07Z

Here, we are ordering just by number whereas bundler-api orderes versions with created_at, number, platform, and dependencies by name (I hope I got this right!).
Do we need all of that? Do we know someone who can answer it?

@fotanus git blame says its you 🔮

dwradcliffe · 2016-05-25T12:05:08Z

@indirect @segiddins might know

fotanus · 2016-05-25T13:01:19Z

I'm in a run right now, but the orderings should be consistent across both bundler-api and rubygems.org. If is not the case, it is wrong.

arthurnn · 2016-05-25T14:48:13Z

Regarding data leaking between tests. We should be using transactional fixtures , which should not let data leaky. Can you check if we have a test that disable it, and dont clean the data afterwards?

arthurnn · 2016-05-25T14:49:27Z

Here, we are ordering just by number whereas bundler-api orderes versions with created_at, number, platform, and dependencies by name (I hope I got this right!).
Do we need all of that?

@sonalkr132 I think it is a good idea to maintain that order.

fotanus · 2016-05-25T14:58:25Z

Now with a bit more of time, I think that we should keep the most restrictive order possible, trying to avoid a database update to break the order and cause trouble. So I suggest copy the order from bundler-api. I have something pending already for bundler, so if you guys are in rush feel free to fix this!

sonalkr132 · 2016-05-25T15:32:18Z

I suggest copy the order from bundler-api.

I am getting more and more inclined towards using raw sql in fetch_dependency_from_db. That way we can almost copy the exact query from bundler-api. I also ran performance test on dependency end point and fetch_dependency_from_db took 27% of request time with one record in DB(10.96% of 40% request time. 60% was spent in setup).

report: https://jsfiddle.net/pudg03g7/

fotanus · 2016-05-25T15:36:34Z

I think @dwradcliffe can help with the code style. I'm good either way.

arthurnn · 2016-05-25T16:02:10Z

@sonalkr132 I am totally for it to use raw SQL here.. As long as we sanitize the parameters for security reasons, thats what we should do IMO.

sonalkr132 · 2016-05-26T15:32:25Z

I have updated it to the exact code of bundler-api, except I have dropped ordering of nested SELECT query. We were ordering again at end so nested ordering didn't make any difference other than slowing the query.

This has one more difference, angle brackets > used in requirements are rendered as ["rake","\u003e= 0.8.3"]. It is not introduced in this PR, rather it was always there. I suppose thats rails default escaping. This stackoverflow answer suggests that it won't make a difference:

The various gsub calls are forcing non-ASCII UTF-8 to the \uXXXX notation that you're seeing. Hex encoded UTF-8 should be acceptable to anything that processes JSON but you could always post-process the JSON (or monkey patch in a modified JSON escaper) to convert the \uXXXX notation to raw UTF-8 if necessary.

homu · 2016-05-28T03:29:21Z

☔ The latest upstream changes (presumably #1293) made this pull request unmergeable. Please resolve the merge conflicts.

homu · 2016-05-29T02:13:05Z

☔ The latest upstream changes (presumably #1294) made this pull request unmergeable. Please resolve the merge conflicts.

raw sql insures ordering consistent with bundler-api, ie versions are ordered by created_at, number(version) and platform, and deps are ordered by name. Add tests for filtering of indexed versions and listing of dependency.

dwradcliffe · 2016-05-31T13:02:38Z

          'ruby_version'      => '>= 2.0.0',
          'checksum'          => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=',
-          'created_at'        => '2016-05-24T00:00:00.000Z',
+          'created_at'        => '2016-05-24 00:00:00',


This is closer but still not exactly what the current format is.
Currently I'm seeing: 2016-05-24 00:00:00 +0000

sonalkr132 force-pushed the dep_fixes branch 7 times, most recently from 3861aa3 to 3ca7268 Compare May 23, 2016 01:55

dwradcliffe added this to the Dependency API Migration milestone May 24, 2016

sonalkr132 force-pushed the dep_fixes branch from 3ca7268 to 7f9a1d2 Compare May 24, 2016 16:57

sonalkr132 force-pushed the dep_fixes branch 2 times, most recently from 9eaada5 to 6241ec8 Compare May 26, 2016 15:21

sonalkr132 force-pushed the dep_fixes branch 3 times, most recently from 9689eae to 0d99580 Compare May 28, 2016 07:43

sonalkr132 changed the title ~~Remove reverse order and show only index version~~ correct versions order and show only indexed versions May 28, 2016

Use raw sql in fetch_dependency_from_db and add tests

9faa01f

raw sql insures ordering consistent with bundler-api, ie versions are ordered by created_at, number(version) and platform, and deps are ordered by name. Add tests for filtering of indexed versions and listing of dependency.

sonalkr132 force-pushed the dep_fixes branch from 0d99580 to 9faa01f Compare May 29, 2016 04:15

dwradcliffe reviewed May 31, 2016
View reviewed changes

Correct format of versions' created_at

08687e9

dwradcliffe merged commit 86062c0 into rubygems:master Jun 1, 2016

dwradcliffe temporarily deployed to staging June 1, 2016 02:35 Inactive

dwradcliffe temporarily deployed to production June 1, 2016 02:58 Inactive

sonalkr132 deleted the dep_fixes branch May 20, 2017 10:08

Uh oh!

Conversation

sonalkr132 commented May 22, 2016

Uh oh!

homu commented May 22, 2016

Uh oh!

sonalkr132 commented May 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

homu commented May 24, 2016

Uh oh!

dwradcliffe commented May 25, 2016

Uh oh!

sonalkr132 commented May 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwradcliffe commented May 25, 2016

Uh oh!

fotanus commented May 25, 2016

Uh oh!

arthurnn commented May 25, 2016

Uh oh!

arthurnn commented May 25, 2016

Uh oh!

fotanus commented May 25, 2016

Uh oh!

sonalkr132 commented May 25, 2016

Uh oh!

fotanus commented May 25, 2016

Uh oh!

arthurnn commented May 25, 2016

Uh oh!

sonalkr132 commented May 26, 2016

Uh oh!

homu commented May 28, 2016

Uh oh!

homu commented May 29, 2016

Uh oh!

dwradcliffe May 31, 2016

Choose a reason for hiding this comment

Uh oh!

sonalkr132 May 31, 2016

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sonalkr132 commented May 23, 2016 •

edited

Loading

sonalkr132 commented May 25, 2016 •

edited

Loading