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

Upgrade to Scanamo v1.1.1 #1351

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Upgrade to Scanamo v1.1.1 #1351

merged 1 commit into from
Jun 11, 2024

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Jun 5, 2024

Here we're moving up from Scanamo 1.0.0-M11 to 1.1.1, which involves several changes to requirements:

See also:

Here we're moving up from Scanamo [1.0.0-M11](https://github.com/scanamo/scanamo/releases/tag/v1.0.0-M11) to [1.1.1](https://github.com/scanamo/scanamo/releases/tag/v1.1.1), which involves several changes to requirements:

* AWS SDK v1 → v2 - Scanamo [1.0-M13](https://github.com/scanamo/scanamo/releases/tag/v1.0-M13) and above require AWS SDK v2. AWS SDK v2 is actually used quite widely already in Riff Raff, so this wasn't hard - there does still seem to be _some_ other code in Riff Raff using AWS SDK v1 at the moment.
* Scanamo API changes:
  - Auto-generated `DynamoFormat` handlers are now created with `org.scanamo.generic.auto._` rather than `org.scanamo.auto._` (scanamo/scanamo#538 - [1.0-M12](https://github.com/scanamo/scanamo/releases/tag/v1.0.0-M12))
  - `DynamoFormat.coercedXmap()` now has a single set of two arguments, rather than curried arguments (scanamo/scanamo#588 - [1.0-M13](https://github.com/scanamo/scanamo/releases/tag/v1.0-M13))
  - `Table.put()` now returns `Unit` rather than the old value- you can use `Table.putAndReturn()` if you want a value returned, which allows you to specify whether it's the prior value or the new one you want (scanamo/scanamo#486 - [1.0-M13](https://github.com/scanamo/scanamo/releases/tag/v1.0-M13))

See also:

* guardian/media-atom-maker#1143 - upgrading to Scanamo v1.0.0-M28
@rtyley rtyley marked this pull request as ready for review June 5, 2024 15:59
@rtyley rtyley requested review from a team as code owners June 5, 2024 15:59
Comment on lines -16 to +17
Trigger.withName
)(_.toString)
Trigger.withName,
_.toString
)
Copy link
Member Author

Choose a reason for hiding this comment

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

DynamoFormat.coercedXmap() now has a single set of two arguments, rather than curried arguments (due to scanamo/scanamo#588 - released with 1.0-M13)

Comment on lines -45 to 44
): Option[Either[DynamoReadError, TargetId]] =
): Unit =
exec(table.put(TargetId(target, projectName, lastSeen)))
Copy link
Member Author

@rtyley rtyley Jun 11, 2024

Choose a reason for hiding this comment

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

Table.put() now returns Unit rather than the old value- you can use Table.putAndReturn() if you want a value returned, which allows you to specify whether it's the prior value or the new one you want (scanamo/scanamo#486 - release with 1.0-M13)

Copy link
Contributor

Choose a reason for hiding this comment

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

I presume this will throw an exception if the update fails, but this seems safe given that our only usage of this function is here:

Either.catchNonFatal(
t -> targetDynamoRepository.set(t, build.jobName, build.startTime)
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think it would always have thrown an exception if the update fails, the difference is just that if the update succeeds, there's now no value being returned, and no chance of a DynamoReadError representing a deserialisation error. And you're right, TargetResolver is catching any erxception!

@rtyley rtyley merged commit 5449b83 into main Jun 11, 2024
1 check passed
@rtyley rtyley deleted the upgrade-scanamo branch June 11, 2024 11:28
@rtyley
Copy link
Member Author

rtyley commented Jun 11, 2024

This was deployed to PROD as build 3392, and subsequent deploys look good:

image

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.

2 participants