feral icon indicating copy to clipboard operation
feral copied to clipboard

feat: add smithy4s AWS types (AwsRegion, AwsArn)

Open najuna-brian opened this issue 2 months ago • 6 comments

Description

Integrates smithy4s for type-safe AWS regions and ARNs.

  • Added smithy4s-core dependency
  • Created AwsRegion
  • Created AwsArn with parsing uti

Refs #118

najuna-brian avatar Oct 15 '25 15:10 najuna-brian

@armanbilge @bpholt @djspiewak Would love your feedback when you have a chance to review! Thank you :blush:

najuna-brian avatar Oct 15 '25 16:10 najuna-brian

Can you discuss the motivation for this a little bit? In particular, why pull in smithy4s-core here? I like the idea of having newtypes for AwsRegion and AwsArn but I'm a little nervous about the new dependency.

bpholt avatar Oct 15 '25 17:10 bpholt

Thank you @bpholt

The motivation is from the issue description addressing @armanbilge 's request:

"Notably, they provide models for some of the basic AWS types e.g. regions, etc. I'll be thrilled to outsource those."

najuna-brian avatar Oct 15 '25 19:10 najuna-brian

Regarding the dependency concerns - Based on some research, I think smithy4s-core fits best because it provides:

  • Type safety & schema support: Built-in Newtype pattern and serialization schemas.
  • Ecosystem fit: as it already recommended in feral’s README (line 67) for AWS clients and aligns with AWS Scala libraries.
  • Lightweight: ~2 MB, minimal deps, cross-platform (JVM + JS).
  • Future-ready: Supports the “persistent chat server” idea and future smithy4s clients.

As an alternative, we could implement the newtypes using feral’s sealed abstract classes, but that would mean losing schema integration and ecosystem benefits what do you think about that trade-off?

najuna-brian avatar Oct 15 '25 19:10 najuna-brian

The motivation is from the issue description addressing @armanbilge 's request:

"Notably, they provide models for some of the basic AWS types e.g. regions, etc. I'll be thrilled to outsource those."

To clarify, my suggestion is that we directly use e.g. AwsRegion from smithy4s, not that we re-implement it here.

I like the idea of having newtypes for AwsRegion and AwsArn but I'm a little nervous about the new dependency.

Yes, I think adding this dependency is a valid concern.

armanbilge avatar Oct 16 '25 02:10 armanbilge

@armanbilge @bpholt Thank you for the clarification! I misunderstood the original intent True! It's better to use smithy4s's existing AwsRegion instead of re-implementing it

Could we Use smithy4s-aws-kernel for AwsRegion and/or Create minimal AwsArn using feral's patterns (since smithy4s doesn't have it)

Would love to be guided on the next step or best approach to take moving forward Thank you :blush:

najuna-brian avatar Oct 16 '25 05:10 najuna-brian