AWS Transfer Family

[armichaud]

Add Transfer client APIs requested in #7790

Implements create_user, describe_user, delete_user, import_ssh_public_key, delete_ssh_public_key, create_server, describe_server, delete_server

[armichaud]

@bblommers this should be good to go

[codecov]

Codecov Report

Attention: Patch coverage is 95.03817% with 13 lines in your changes missing coverage. Please review.

Project coverage is 94.42%. Comparing base (b6bb11e) to head (d786722).
Report is 46 commits behind head on master.

Files	Patch %	Lines
moto/transfer/models.py	88.17%	11 Missing ⚠️
moto/transfer/exceptions.py	83.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7891      +/-   ##
==========================================
+ Coverage   94.35%   94.42%   +0.06%     
==========================================
  Files        1113     1115       +2     
  Lines       94650    95609     +959     
==========================================
+ Hits        89308    90278     +970     
+ Misses       5342     5331      -11     

Flag	Coverage Δ
servertests	29.05% <0.38%> (-0.17%)	⬇️
unittests	94.39% <95.03%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[armichaud]

@bblommers I have refactored everything such that the backend converts everything to snake_case and back to camel case.

Just a bit of feedback on that pattern: I understand it's the convention for the repo, but I'm not sure what it adds besides boilerplate.

Moreover, the linter is super aggressive, and at times I wasn't sure what its rules really accomplished. For example, it threw type errors at things like this:

server = {...}
if self.attr is not None:
    server["attr"] = self.attr 

but had no problem with this:

attr = {}
if self.attr is not None:
    attr = self.attr
server = {..., "attr": attr}

The volume of linting issues made it just too annoying to work with TypedDicts, so I scrapped them and just made most of the type hints generic.

[bblommers]

This looks great - thank you for adding this to Moto @armichaud!

The snake case is indeed just convention, but does make development easier when switching between different services - there's less of a mental effort in trying to understand the code. It's also easier to read IMO, but I understand that's a personal preference.

The typing - that's indeed a bit of a pain. In that particularly example, I've learned that mypy likesself.attr = attr or {} best, but that is just experience. Overall, mypy has caught (and still does catch) quite a few number of bugs, so it is worth it IMO.

[moto-payments-app]

Hi @armichaud,

To show our thanks, we'd like to share some of the donations that we've received with you. PR's like this are the big reason that Moto is as successful as it is - so it's only fair that you, as a contributor, get to share the spoils.

We've created a companion website with more information:
https://payments.getmoto.org/

Feel free to open a bug or discussion if you run into any problems:
https://github.com/getmoto/payments

[jenstroeger]

Thank you @armichaud, I’ll test more tomorrow!

To show our thanks, we'd like to share some of the donations that we've received with you. PR's like this are the big reason that Moto is as successful as it is - so it's only fair that you, as a contributor, get to share the spoils.

That’s a great idea, well deserved!

[github-actions]

This is now part of moto >= 5.0.13.dev12

[armichaud]

This looks great - thank you for adding this to Moto @armichaud!

The snake case is indeed just convention, but does make development easier when switching between different services - there's less of a mental effort in trying to understand the code. It's also easier to read IMO, but I understand that's a personal preference.

The typing - that's indeed a bit of a pain. In that particularly example, I've learned that mypy likesself.attr = attr or {} best, but that is just experience. Overall, mypy has caught (and still does catch) quite a few number of bugs, so it is worth it IMO.

Makes sense! Consistency is definitely valuable, and I agree that the headache with linting is probably worth the bugs that it catches. I suspect that there are reasons for those preferences that have to do with the subtleties of typing in python that I'm just not as familiar with.

And thank you for the donation sharing!

[jenstroeger]

I started testing and one issue I came across so far:

diff --git a/moto/transfer/models.py b/moto/transfer/models.py
index 360b82549..ca9f5e69e 100644
--- a/moto/transfer/models.py
+++ b/moto/transfer/models.py
@@ -131,7 +131,7 @@ class TransferBackend(BaseBackend):
         self,
         home_directory: Optional[str],
         home_directory_type: Optional[UserHomeDirectoryType],
-        home_directory_mappings: List[Dict[str, Optional[str]]],
+        home_directory_mappings: Optional[List[Dict[str, Optional[str]]]],
         policy: Optional[str],
         posix_profile: Optional[Dict[str, Any]],
         role: str,
@@ -150,7 +150,7 @@ class TransferBackend(BaseBackend):
             tags=(tags or []),
             user_name=user_name,
         )
-        if len(home_directory_mappings) > 0:
+        if home_directory_mappings:
             for mapping in home_directory_mappings:
                 user.home_directory_mappings.append(
                     {

For create_user() that argument is optional (because it’s not labeled “[REQUIRED]” like e.g. the Role argument).