Aquarius returns 500 for /ddo/query

[TimDaub]

URL: POST https://aquarius.mainnet.oceanprotocol.com/api/v1/aquarius/assets/ddo/query
Headers:

{
  "Content-Type": "application/json",
  Accept: "application/json",
  "User-Agent": "rugpullindex.com crawler/0.0.1"
}

Body:

{
  page: 1,
  offset: 407,
  query: { 'price.type': [ 'pool' ] },
  sort: { 'price.ocean': -1 }

The answer is 500 INTERNAL SERVER ERROR.

I had this in one of my tests and today was the first time it failed.

[calina-c]

A new version of Aquarius 2.2.6 has been released and fixes some issues with queries.

Recently we've been working on adding more flexibility to ES queries and supporting them by default. The "query" field of this payload must now respect ElasticSearch syntax e.g. the query key should contain:

"bool": { "must": [ {"match": {"price.type": "pool"}} ] }

in order to retrieve all entries with price.type == "pool".

Per ES docs: https://www.elastic.co/guide/en/elasticsearch/reference/6.6/query-dsl.html

[TimDaub]

My assumption is that a breaking change like this would require updating the API's major version within the API URL, no?

https://aquarius.mainnet.oceanprotocol.com/api/v1/aquarius/assets/ddo/query

would become

https://aquarius.mainnet.oceanprotocol.com/api/v2/aquarius/assets/ddo/query

[calina-c]

We discussed with the team and unfortunately we can not afford to maintain two versions right now. :(

I'm closing this ticket, since the query works properly.

[TimDaub]

Maybe I've expressed myself wrongly in my last comment. Truely I don't care much about maintaining the v1 endpoint. It could be deprecated with some reasonable notice period and then switch to v2.

What I'm asking is if you can implement proper semantic versioning in the API url. For this you won't have to maintain any other version but the latest. All I'm asking is to up the version numberat a breaking change.

Otherwise you might as well remove "v1" from the API path completely since it serves no purpose.

[kremalicious]

I second that, I do not know what the purpose of that v1 in the api path is. Or even why there is aquarius in the API path. It's all never thought through legacy code. But this opened issue shows we absolutely need something to signal breaking changes. API versioning is one solution, sticking to actual semantic versioning with documented changes another one.

If we do not intend to switch API versions, then we should remove the versioning to not set any wrong expectations. Likewise, none of the breaking updates are ever documented. On the market we often find ourselves scrambling to update because some new minor Aquarius update (based on version number) was deployed and was actually a breaking change and should have been a major new release in semver terms.

When then going to this repo to figure out what changed:

• Changelog.md is stuck at 2 years ago so it's abandoned
• release notes in GitHub releases do not document any changes, or warnings, or migrations

Like, this is the release which introduced the breaking search changes reported in this issue: https://github.com/oceanprotocol/aquarius/releases/tag/v2.2.5

For outside world this was a patch release with no breaking changes and it's impossible to know what changed unless users start some heavy git comparisons. So for anybody not developing Aquarius it's almost impossible to know if there is a breaking change.

[calina-c]

@kremalicious @trentmc should I reopen this issue or should we create a new one after we establish a concrete list of Todos?