Fix cloud resource naming #1099
Conversation
| @@ -8,9 +8,9 @@ | |||
| coverage/ | |||
|
|
|||
| # Config files | |||
There was a problem hiding this comment.
It would be nice to mention that this is deliberate so the next me won't update this.
| @@ -18,12 +18,12 @@ export async function run(...args: unknown[]) { | |||
| return handler.run(...args); | |||
| } | |||
|
|
|||
| export async function processHttpRequest(...args: unknown[]) { | |||
| export async function handleHttpReq(...args: unknown[]) { | |||
There was a problem hiding this comment.
Do we want to add a comment explaining why is the fn name abbreviated? We are consistent in naming in Airnode and this is tempting to rename...
There was a problem hiding this comment.
Nit: Also maybe better name would be httpRequest (dropping the "handle" part)
There was a problem hiding this comment.
Actually, after thinking about this more I dislike that the "function name length" pollutes the deployer handlers... I'd propose the following. Create a file entrypoint which will be called by TF and rename the current index file to implementation.
The entrypoint file should be documented that it exist because the function name length is restricted, but the implementation can be named freely.
There was a problem hiding this comment.
Sorry, I don't follow. But I can rename the functions just here, in entrypoint.ts, and leave the handlers (those in aws and gcp directories) as they were. But that feels even less consistent.
There was a problem hiding this comment.
Sorry, I missed that we already have the entrypoint.ts file. I'd change the function names in this file and document that this is needed because of GCP/AWS.
This allows us to deal with that problem only in entrypoint and use whatever name we want in the rest of the codebase.
amarthadan
force-pushed
the
fix-cloud-resource-naming
branch
from
0c7dd1b
to
0c1db9c
Compare
| @@ -18,12 +18,13 @@ export async function run(...args: unknown[]) { | |||
| return handler.run(...args); | |||
| } | |||
|
|
|||
| export async function processHttpRequest(...args: unknown[]) { | |||
| // We shorten function names to allow for shorter cloud resource names | |||
There was a problem hiding this comment.
I'd either copy this command above every function (preferred) or make it a top level command.
There was a problem hiding this comment.
👍 Left one commend https://github.com/api3dao/airnode/pull/1099/files#r875532041 but therwise LGTM
amarthadan
force-pushed
the
fix-cloud-resource-naming
branch
from
0c1db9c
to
d3cb0fd
Compare
Issue AN-737
I've introduced two naming changes:
-roleto the role name as it's under roles in the dashboard)processHttpSignedDataRequesttohandleHttpSignedReqandhttpSignedDataGatewaytohttpSignedGw. I changed this only for the cloud resource names and not throughout the whole codebase because I would ideally have to change also the fields in the configuration. I know these names are not ideal, I'm open to suggestions, I'm not good at naming stuff 😄The longest names for cloud resources possible with these changes are:
airnode-0123456-0123456789abcdef-handleHttpSignedReq-schedule(61 characters)airnode-0123456-0123456789abcdef-httpSignedGw-lambda-invoke(59 characters)for AWS and
airnode-0123456-0123456789abcdef-handleHttpSignedReq-scheduler(62 characters)airnode-0123456-0123456789abcdef-httpSignedGw(45 characters)for GCP which is within the common maximal 64 characters restriction.