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

migrate [MozillaObservatory] to new API #10402

Merged
merged 3 commits into from
Jul 26, 2024
Merged

Conversation

chris48s
Copy link
Member

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:

  • The API seems to no longer explicitly issue a scan status. Dealing with a pending scan, for example, doesn't really seem to be a thing any more. I don't really have the examples to easily test what all the failure cases look like, although I did think of one obvious one. I think in lieu of better docs, we can say we either:
    • Get a scan result or
    • The validation fails and we reject the response with the generic "invalid" message
      We can always improve the error messages in future when we have docs to understand the failure cases a bit better.
  • The concept of publishing (or not) the result seems to be gone.
  • I guess this could be a service where the first request sometimes takes a long time, but once the first scan has happened we have a cached result. I haven't done it here, but I wonder if this would be a good place to use Implement a pattern for dealing with upstream APIs which are slow on the first hit; affects [endpoint] #9233
  • This service had a bunch of tests which were done using mocks. I decided to bin them and switch those to testing the rendering function in unit tests in a .spec file

@chris48s chris48s added the service-badge New or updated service badge label Jul 23, 2024
Copy link
Contributor

github-actions bot commented Jul 23, 2024

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 69bb645

@yestalgia
Copy link

Thank you for your quick action on this one. It's greatly appreciated.

@nicwortel
Copy link

Thank you for creating this PR!

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.

For me it still appears to be broken:

afbeelding

PyvesB
PyvesB previously approved these changes Jul 25, 2024
Copy link
Member

@PyvesB PyvesB left a 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)
Copy link
Member

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?

Copy link
Member Author

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 😎

@chris48s chris48s merged commit da0fd70 into badges:master Jul 26, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MDN HTTP Observatory is launched, and Mozilla Observatory is now deprecated.
4 participants