Update to fapi-client v4.0.6

[rtyley]

This is a small upgrade, catching up with the recent dependency updates of https://github.com/guardian/facia-scala-client/releases/tag/v4.0.6, before the more extensive update in guardian/facia-scala-client#287 is introduced.

This update has already been tested in Ophan's PromotionPoller with https://github.com/guardian/ophan/pull/5540, successfully deployed to Prodution.

[Georges-GNM]

This one should be good too 🙂

[prout-bot]

Overdue on PROD (merged by @rtyley 30 minutes and 2 seconds ago) What's gone wrong?

[rtyley]

Overdue on PROD (merged by @rtyley 30 minutes and 2 seconds ago) What's gone wrong?

The GitHub CI job for the merged main commit 10b6b60 originally failed with what looks to be a flakey test:

    ✗ Allows the same alert to be sent multiple times
	Failed: edit action timeout, endpoint /edits
	Error: edit action timeout, endpoint /edits
	    at eval (http://localhost:9876/test/utils/actions/base-action.js!transpiled:34:50)

...this test passed when it was originally run on the PR branch, and then passed when the main-branch CI was re-run.

As it turns out - the main commit originally failing CI and not getting deployed was actually fortuitous, because there really is a problem with this PR, just not anything the current tests would show - a NoSuchMethodError exception (full stacktrace, logs), probably with a cause similar to, eg guardian/support-frontend#609 - versions of FAPI, CAPI, and Circe getting out of sync 😢

This probably wasn't seen with https://github.com/guardian/ophan/pull/5540, guardian/frontend#26578 or https://github.com/guardian/apple-news/pull/263 because those projects happened to already have a compatible set of versions for those dependencies (those projects all use Scala Steward, so are all using the latest versions). However, it was seen with guardian/story-packages#184, where it caused an outage (arrgh).

I'm reverting this PR as it's not safe to deploy - I'll try to create a new PR with a test (perhaps like the tests added in https://github.com/guardian/ophan/pull/4719 or guardian/frontend#25139?.

[prout-bot]

Seen on PROD (merged by @rtyley 309 hours, 8 minutes and 16 seconds ago) Please check your changes!

[twrichards]

Seen on PROD (merged by @rtyley 309 hours, 8 minutes and 16 seconds ago) Please check your changes!

@rtyley any idea why this was so slow to roll out? Curious timing as there's a CP email thread about search results in the story packages tool

[davidfurey]

versions of FAPI, CAPI, and Circe getting out of sync 😢

It isn't unusual for versions of FAPI and CAPI to get out of sync and for this to cause run time errors. Is there any easy way we can consider evictions to be a build failure but only for specific libraries?

[rtyley]

@rtyley any idea why this was so slow to roll out?

I think the message by Prout yesterday at 3:14pm on 26th September ("Seen on PROD (merged by @rtyley 309 hours, 8 minutes and 16 seconds ago)") is misleading, in terms of its timing. In truth, this PR (with commit 10b6b60) would finally have 'made it' to Production when #1527 went out with commit 256f5c8, reverting this PR, on 14th September (deploy). It's a failing on Prout's part that it didn't immediately report the PR as live when it went out on 14th September - sometimes it won't rescan a repo often enough after a PR to catch the eventual deployment.

So far as I'm aware, nothing actually changed in PROD https://fronts.gutools.co.uk/v2 yesterday, except that Prout finally did a follow-up scan and realised that the changes were present. Prout's fresh scan yesterday was triggered by our GitHub guardian organisation webhook for Prout, which asks Prout to rescan repos when PRs are merged or re-labelled - and as it happens, I added a Europe Editon label to the https://github.com/guardian/facia-tool repository yesterday - even though that was on a different PR, that was enough to trigger a Prout rescan:

Curious timing as there's a CP email thread about search results in the story packages tool

I can't explain that, I think it's probably a coincidence?

[twrichards]

which asks Prout to rescan repos when PRs are merged or re-labelled - and as it happens, I added a Europe Editon label to the guardian/facia-tool repository yesterday - even though that was on a different PR, that was enough to trigger a Prout rescan

this explains it @rtyley - thanks for the detailed explanation

Curious timing as there's a CP email thread about search results in the story packages tool

I can't explain that, I think it's probably a coincidence?

yes definitely a coincidence now you've explained the Europe Edition labelling (story packages CP email thread was loosely related anyway)

[rtyley]

It isn't unusual for versions of FAPI and CAPI to get out of sync and for this to cause run time errors. Is there any easy way we can consider evictions to be a build failure but only for specific libraries?

Good question @davidfurey, yep, there is! I'm currently looking into this... here's some background, but in summary, you need all 3 of these things:

• Easy : Use sbt 1.5.0 or higher everywhere (well, we're probably already doing that)
• Easy : Add ThisBuild / versionScheme := Some("early-semver") configuration to libraries we want to provide protection for (eg facia-scala-client, content-api-scala-client, content-api-models, etc)
• Hard : ensure that library updates are released with correctly incremented major.minor.patch version number updates, that respect Scala's Recommended Versioning Scheme, which is an extended set of rules adding on to plain semver.

With all this in place, sbt will correctly fail the build with an error when, say version 1.1.0 of a library is evicted in preference of version 1.2.0.

It's not happening at the moment because we don't have the versionScheme configuration, but even with that, humans don't have a good intuition about when a change is binary incompatible, and tend to make releases with patch version increments when major is actually required (for instance, the upgrade that caused this issue with the FAPI client was content-api-models v17.5.1 → v17.5.2 which was an innocent looking change, but actually binary incompatible, and so should have been a major update - 'major' updates would probably be a lot more common with this scheme).

There's a ScalaCenter sbt plugin called sbt-version-policy that aims to enforce correct library version numbering, using MIMA & dependency analysis - if you're developing facia-scala-client 4.1.7-SNAPSHOT, it will download the previous published artifact (v4.1.6), and work out if your compatibility is what you think it is. Having run it, I can see that it would have caught the issue with content-api-models v17.5.1 → v17.5.2, which is good, but I would like to improve the workflow to make it easier to use - I've opened draft PR scalacenter/sbt-version-policy#179 to try to explain what I think would be a good flow.

If you'd like to have a chat about this, let me know, would love to talk about this! Doc with proposal.

[davidfurey]

Thanks for the detailed explanation. Happy to talk about this too