-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
migrate [MozillaObservatory] to new API #10402
Conversation
Thank you for your quick action on this one. It's greatly appreciated. |
Thank you for creating this PR!
For me it still appears to be broken: |
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.
Thanks for the migration!
.required() | ||
|
||
t.create('request on observatory.mozilla.org') | ||
t.create('valid') | ||
.timeout(10000) |
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.
Are the timeouts still relevant with the new API?
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.
It is a good question. Anecdotally this new service does seem to be faster, but this probably depends a bit on how quickly the domain you're scanning (which is upstream again from mozilla) responds.
I think I'm going to go ahead and merge this as the old API seems to be down again. I've pushed a commit which removes the timeouts. Lets keep an eye on the daily test runs. If this ends up being flaky, we can put them back. If it is stable, then cool 😎
Closes #10396
It seems like the old API is now back online and the badge is working again, so we don't have to merge this immediately. We can take a bit of time to wok out the details. But it is deprecated and due to be retired in September so we do need to migrate this soon-ish.
This is a bit tricky to work on as the API is currently undocumented (although we have been told we can use it and docs are coming). There are some behaviours that aren't exactly specified. I've made some assumptions. Some notes:
We can always improve the error messages in future when we have docs to understand the failure cases a bit better.
.spec
file