-
Notifications
You must be signed in to change notification settings - Fork 38
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
Refactored measurement page using refactored API #486
Conversation
uses mock response for upcoming measurement_meta API endpoint
8d0f1c7
to
48806cb
Compare
@sarathms let's take at look at this together later today? 🙏 |
} | ||
|
||
Measurement.propTypes = { | ||
measurement: PropTypes.object.isRequired, | ||
measurement: PropTypes.object, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sarathms please make sure we list all the props here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks good to me. We should probably take another look at it later today, to exchange further ideas, but it's good work and definitely on the right track. Thank you!
We're aiming for writing a full end to end experiment where the code performs measurements and then fetches them back from API. However, the API code currently does not store measurements submitted by us using ams-pg.ooni.org, so we cannot go that far. Yet, let us have support for this functionality, which has been tested by adapting ooni/explorer#486. Also, for now just write basic smoke testing. The diff itself details a plan for integrating this code into probe-engine. When we'll do that we'll most likely want to write more testing code. This work is part of ooni/backend#446.
We're aiming for writing a full end to end experiment where the code performs measurements and then fetches them back from API. However, the API code currently does not store measurements submitted by us using ams-pg.ooni.org, so we cannot go that far. Yet, let us have support for this functionality, which has been tested by adapting ooni/explorer#486. Also, for now just write basic smoke testing. The diff itself details a plan for integrating this code into probe-engine. When we'll do that we'll most likely want to write more testing code. This work is part of ooni/backend#446.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should merge now and without reverting anything. Because we only deploy releases, our master branch represents a development snapshot towards the goal we have in mind for the next release. Therefore, I believe it's in our best interest to bring this good set of changes to master such that we can immediately start using them along with others.
🐳
Fixes #489
Uses the new
/api/v1/measurement_meta
API to fetch measurement data.Revert 8d0f1c7 before mergingWe decided to continue to use theams-pg.ooni.org
hostname for now.