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

Wind feed Oracle #28

Merged
merged 1 commit into from
Oct 18, 2019
Merged

Conversation

janus
Copy link
Contributor

@janus janus commented Sep 14, 2019

@Amxx
Copy link
Member

Amxx commented Sep 25, 2019

Results for the run on kovan is a file, this does not match the expected result of an oracle (which results must be encoded so that they are processed onchain)

https://explorer.iex.ec/kovan/task/0xf0881ff55a30eadd44c3dc081b21692b36a963aff457b3be5bc7913bc1da4ac1

Copy link
Member

@Amxx Amxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall very good proposition. An endpoint change is needed to make the result deterministic. Other then that is good for me !


const url = "https://api.openweathermap.org/data/2.5/weather";
const query1 = `${url}?lat=${lat}&lon=${long}&appid=${APIKEY1}`;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good but not deterministic. I would suggest replacing lat/lon with lat/lon/time and use the endpoint:
http://history.openweathermap.org/data/2.5/history/city?lat={lat}&lon={lon}&type=hour&start={start}&cnt={cnt}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amxx
Thanks for your review. But historical data is not for free subscription . https://openweathermap.org/faq#error401
I don't know what to do next.

var nd = new Date(utc + (1000 * results.timezone));


timestamp = nd.getTime();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If moving from lat/lon to lat/lon/time → use the time as a timestamp (deterministic)

@sulliwane sulliwane merged commit 890b1ff into iExecBlockchainComputing:master Oct 18, 2019
@sulliwane
Copy link
Collaborator

Ok @janus, that will work with live data then! great work by the way. Expect the bounties to be released in two weeks maximum (hopefully before that), thanks!

@janus
Copy link
Contributor Author

janus commented Oct 19, 2019

@sulliwane
Let me know when the time comes so that I would re-apply. I have limited gitcoin engagement rights .. because of the time this bounty has taken.. I removed myself to use it on other bounties

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.

3 participants