-
Notifications
You must be signed in to change notification settings - Fork 0
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
CCM-7465: implement backend api PT.2 #260
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,18 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Conversation.
return NextResponse.next(); | ||
} | ||
|
||
const token = await getAccessTokenServer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See conversation
@@ -333,7 +333,7 @@ | |||
"authorizerUri": "arn:aws:apigateway:${AWS_REGION}:lambda:path/2015-03-31/functions/${AUTHORIZER_LAMBDA_ARN}/invocations", | |||
"authorizerCredentials": "${APIG_EXECUTION_ROLE_ARN}", | |||
"identitySource": "method.request.header.authorization,context.resourceId,context.httpMethod", | |||
"authorizerResultTtlInSeconds": 300 | |||
"authorizerResultTtlInSeconds": 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harrim91 - This is where I'm disabling the caching for now.
It was causing me issues and for the sake of getting this PR in I'll leave it disabled. It sounds like you've potentially got a fix for it in your upcoming PR?
@@ -4,9 +4,21 @@ import baseConfig from './playwright.config'; | |||
export default defineConfig({ | |||
...baseConfig, | |||
|
|||
timeout: 10_000, | |||
|
|||
timeout: 30_000, // 30 seconds in the playwright default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've increased these timeouts because creating a new sandbox the lambdas have a cold start which cause the pages to load slowly.
In some cases I've seen a page load take up to 8 seconds (this can be seen on a failed run via the traces). This seems to be combination of factors
- Page load (next SSR'd the page on first load takes ~2-3s)
- API call on a cold start takes ~3-4s
After initial loads the app seems okay but still somewhat sluggish.
@@ -84,7 +84,7 @@ test.describe('View submitted Email message template Page', () => { | |||
|
|||
await assertSkipToMainContent(props); | |||
await assertNotifyBannerLink(props); | |||
await assertLoginLink(props); | |||
await assertLogoutLink(props); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've replaced all of the assertLoginLink
tests in favor of adding in the assertLogoutLink
. We have a ticket for negative testing CCM-7467
@@ -32,7 +32,7 @@ runs: | |||
run: zip lines-of-code-report.json.zip lines-of-code-report.json | |||
- name: "Upload CLOC report as an artefact" | |||
if: ${{ !env.ACT }} | |||
uses: actions/upload-artifact@v3 | |||
uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub was failing due to V3 being depreciated.
sandbox-set-up: | ||
name: "Sandbox set up" | ||
runs-on: ubuntu-latest | ||
runs-on: ubuntu-22.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #259
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the above PR is merged into main I'll revert this change.
da0c4ae
to
cbf9d1c
Compare
Description
I closed the previous PR since it had over 200 items in the conversation. I've copied over the questions.
PT.3 will remove amplify entirely.
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.