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

Endpoint `/proposals?fetchQuality=true` merges Discovery + Quality Oracle data #429

Closed
Waldz opened this issue Oct 8, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@Waldz
Copy link
Member

commented Oct 8, 2018

  • As endpoint /quality returns known data - proposals, which has statistics
  • This endpoint merges Quality Oracle data into proposal, even if we dont have this data
  • Implement query filter fetchQuality=true, which forces to returns quality structures in response
    "proposals": [
        {
            "id": 1,
            "providerId": "0x04e21530fd5114a2db8029694631d61ede835961",
            "serviceType": "openvpn",
            "serviceDefinition": {
                "locationOriginate": {
                    "country": "US"
                }
            },
            "quality": {
                "connects": {
                    "countAll": 100,
                    "countSuccess": 95,
                    "countFail": 0,
                    "countTimeout": 5,
                }
            }
        },
        {
            "id": 2,
            "providerId": "0x04e21530fd5114a2db8029694631d61ede835961",
            "serviceType": "wireguard",
            "serviceDefinition": {
                "locationOriginate": {
                    "country": "US"
                }
            },
            "quality": {
                "connects": {
                    "countAll": 0,
                    "countSuccess": 0,
                    "countFail": 0,
                    "countTimeout": 0,
                }
            }
        },
    ]
}

@Waldz Waldz changed the title Endpoint `/proposals?fetchQuality` merges Discovery + Quality Oracle data Endpoint `/proposals?fetchQuality=true` merges Discovery + Quality Oracle data Oct 8, 2018

@soffokl soffokl self-assigned this Oct 10, 2018

@soffokl

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

I think we can remove count prefix from the block:

         "quality": {
                "connects": {
                    "countAll": 0,
                    "countSuccess": 0,
                    "countFail": 0,
                    "countTimeout": 0,
                }
            }

Since it always repeted and coudn't be anything else.

@soffokl

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

And we can remove all counter too, as @donce suggested.

@zolia

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

I would suggest to rename "quality" -> "metrics". It is for client to decide which metric it will use for its quality calculation - not us.

@zolia

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

Also, theres a bit better logic grouping metrics, saving space and more 'metric' oriented:
Would remake this one:

            "quality": {
                "connects": {
                    "countAll": 0,
                    "countSuccess": 0,
                    "countFail": 0,
                    "countTimeout": 0,
                }
            }

To this one:

            "metrics": {
                "connectCounts": {
                    "Success": 0,
                    "Fail": 0,
                    "Timeout": 0,
                }
            }
@zolia

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

this logic would also assume better naming: /proposals?fetchMetrics=true or some more specific filtering /proposals?fetchMetric=connectCounts.
Not sure about usage of GET here.. maybe json?

@soffokl

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

I like the Metric naming because it's not a real quality.
@Waldz what do you think?

@soffokl

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

About querying different ways it should go to another issue: #424

@Waldz

This comment has been minimized.

Copy link
Member Author

commented Oct 17, 2018

I agree also:

  • lets rename data structure quality -> metrics
  • fetchQuality=true -> fetchConnectCounts=true (no names)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.