-
Notifications
You must be signed in to change notification settings - Fork 1
[DEV-2583] Add optimized GitBook sync workflow and metadata synchronization script #1711
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: c20a346 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
guides: StrapiGuide[]; | ||
solutions: StrapiSolution[]; | ||
releaseNotes: StrapiReleaseNote[]; | ||
guidesResponse?: any; |
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.
Maybe you should find a more explicit name for *Response attributes
}[]; | ||
} | ||
|
||
let s3Client: ReturnType<typeof makeS3Client> | undefined; |
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.
Why don't you use S3Client?
let s3Client: ReturnType<typeof makeS3Client> | undefined; | |
let s3Client: S3Client | undefined; |
|
||
let s3Client: ReturnType<typeof makeS3Client> | undefined; | ||
|
||
function getS3Client(): ReturnType<typeof makeS3Client> { |
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.
Same here
'api/release-notes/?populate[bannerLinks][populate][0]=icon&populate[product][populate][0]=logo&populate[product][populate][1]=bannerLinks.icon&populate[product][populate][2]=overview&populate[product][populate][3]=quickstart_guide&populate[product][populate][4]=release_note&populate[product][populate][5]=api_data_list_page&populate[product][populate][6]=api_data_list_page.apiData.*&populate[product][populate][7]=api_data_list_page.apiData.apiRestDetail.slug&populate[product][populate][8]=api_data_list_page.apiData.apiRestDetail.specUrls&populate[product][populate][9]=api_data_list_page.apiData.apiSoapDetail.*&populate[product][populate][10]=guide_list_page&populate[product][populate][11]=tutorial_list_page&populate[seo][populate]=*,metaImage,metaSocial.image&pagination[pageSize]=1000&pagination[page]=1' | ||
), | ||
// Guide list pages | ||
getResponseFromStrapi( |
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.
You should use the typed function fetchFromStrapi
guides: guidesResult.data, | ||
solutions: solutionsResult.data, | ||
releaseNotes: releaseNotesResult.data, | ||
guidesResponse: guidesResult.responseJson, | ||
solutionsResponse: solutionsResult.responseJson, | ||
releaseNotesResponse: releaseNotesResult.responseJson, | ||
guideListPagesResponse, | ||
solutionListPageResponse, |
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.
Is it necessary to distinguish between data, responseJson and full Response?
async function cachedListS3Files(prefix: string): Promise<string[]> { | ||
if (s3ListCache.has(prefix)) { | ||
return s3ListCache.get(prefix)!; | ||
} | ||
const files = await listS3Files(prefix, S3_BUCKET_NAME!, getS3Client()); | ||
s3ListCache.set(prefix, files); | ||
return files; | ||
} | ||
|
||
async function cachedDownloadS3File(filePath: string): Promise<string> { | ||
if (s3FileCache.has(filePath)) { | ||
return s3FileCache.get(filePath)!; | ||
} | ||
const content = await downloadS3File( | ||
filePath, | ||
S3_BUCKET_NAME!, | ||
getS3Client() | ||
); | ||
s3FileCache.set(filePath, content); | ||
return content; | ||
} |
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.
You should read files from file system
…mprove directory checks, and streamline file reading
Jira Pull Request LinkThis Pull Request refers to the following Jira issue DEV-2583 |
Branch is not up to date with base branch@tommaso1 it seems this Pull Request is not updated with base branch. |
This PR exceeds the recommended size of 800 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
] = await Promise.all([ | ||
// Guides with full populate | ||
fetchFromStrapi<StrapiGuide>( | ||
'api/guides?populate[0]=product&populate[1]=versions&populate[2]=image&populate[3]=mobileImage&populate[4]=bannerLinks&populate[5]=bannerLinks.icon&populate[6]=listItems&populate[7]=seo&populate[8]=seo.metaImage&populate[9]=seo.metaSocial.image&pagination[pageSize]=1000&pagination[page]=1' |
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.
Here we should use Query String's stringify function like in apps/nextjs-website/src/lib/strapi/fetches/fetchGuides.ts
.
By doing so, we could manage this query params by declaring a const like in this example:
populate: {
image: { populate: '*' },
mobileImage: { populate: '*' },
listItems: { populate: '*' },
versions: { populate: '*' },
bannerLinks: { populate: ['icon'] },
seo: { populate: 'metaSocial.image' },
product: {
...productRelationsPopulate,
},
},
};```
and then converting it to a string:
`qs.stringify({
...guidesPopulate,
});`
] = await Promise.all([ | ||
// Guides with full populate | ||
fetchFromStrapi<StrapiGuide>( | ||
'api/guides?populate[0]=product&populate[1]=versions&populate[2]=image&populate[3]=mobileImage&populate[4]=bannerLinks&populate[5]=bannerLinks.icon&populate[6]=listItems&populate[7]=seo&populate[8]=seo.metaImage&populate[9]=seo.metaSocial.image&pagination[pageSize]=1000&pagination[page]=1' |
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.
You should add &populate[product][populate][10]=use_case_list_page
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.
This is the current and updated query:
api/guides/?populate[image][populate]=*&populate[mobileImage][populate]=*&populate[listItems][populate]=*&populate[versions][populate]=*&populate[bannerLinks][populate][0]=icon&populate[seo][populate]=metaSocial.image&populate[product][populate][0]=logo&populate[product][populate][1]=bannerLinks.icon&populate[product][populate][2]=overview&populate[product][populate][3]=quickstart_guide&populate[product][populate][4]=release_note&populate[product][populate][5]=api_data_list_page&populate[product][populate][6]=api_data_list_page.apiData.*&populate[product][populate][7]=api_data_list_page.apiData.apiRestDetail.*&populate[product][populate][8]=guide_list_page&populate[product][populate][9]=tutorial_list_page&populate[product][populate][10]=use_case_list_page
), | ||
// Guide list pages | ||
fetchFromStrapi<StrapiGuideListPageResponse>( | ||
'api/guide-list-pages/?populate%5Bproduct%5D%5Bpopulate%5D%5B0%5D=logo&populate%5Bproduct%5D%5Bpopulate%5D%5B1%5D=bannerLinks.icon&populate%5Bproduct%5D%5Bpopulate%5D%5B2%5D=overview&populate%5Bproduct%5D%5Bpopulate%5D%5B3%5D=quickstart_guide&populate%5Bproduct%5D%5Bpopulate%5D%5B4%5D=release_note&populate%5Bproduct%5D%5Bpopulate%5D%5B5%5D=api_data_list_page&populate%5Bproduct%5D%5Bpopulate%5D%5B6%5D=api_data_list_page.apiData.%2A&populate%5Bproduct%5D%5Bpopulate%5D%5B7%5D=api_data_list_page.apiData.apiRestDetail.%2A&populate%5Bproduct%5D%5Bpopulate%5D%5B8%5D=guide_list_page&populate%5Bproduct%5D%5Bpopulate%5D%5B9%5D=tutorial_list_page&populate%5BguidesByCategory%5D%5Bpopulate%5D%5B0%5D=guides.mobileImage&populate%5BguidesByCategory%5D%5Bpopulate%5D%5B1%5D=guides.image&populate%5BguidesByCategory%5D%5Bpopulate%5D%5B2%5D=guides.listItems&populate%5BbannerLinks%5D%5Bpopulate%5D%5B0%5D=icon&populate%5Bseo%5D%5Bpopulate%5D=%2A%2CmetaImage%2CmetaSocial.image' |
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.
Same for StrapiGuideListPage:
api/guide-list-pages/?populate[product][populate][0]=logo&populate[product][populate][1]=bannerLinks.icon&populate[product][populate][2]=overview&populate[product][populate][3]=quickstart_guide&populate[product][populate][4]=release_note&populate[product][populate][5]=api_data_list_page&populate[product][populate][6]=api_data_list_page.apiData.*&populate[product][populate][7]=api_data_list_page.apiData.apiRestDetail.*&populate[product][populate][8]=guide_list_page&populate[product][populate][9]=tutorial_list_page&populate[product][populate][10]=use_case_list_page&populate[guidesByCategory][populate][0]=guides.mobileImage&populate[guidesByCategory][populate][1]=guides.image&populate[guidesByCategory][populate][2]=guides.listItems&populate[bannerLinks][populate][0]=icon&populate[seo][populate]=*,metaImage,metaSocial.image
), | ||
// Release notes with full populate | ||
fetchFromStrapi<StrapiReleaseNote>( | ||
'api/release-notes/?populate[bannerLinks][populate][0]=icon&populate[product][populate][0]=logo&populate[product][populate][1]=bannerLinks.icon&populate[product][populate][2]=overview&populate[product][populate][3]=quickstart_guide&populate[product][populate][4]=release_note&populate[product][populate][5]=api_data_list_page&populate[product][populate][6]=api_data_list_page.apiData.*&populate[product][populate][7]=api_data_list_page.apiData.apiRestDetail.slug&populate[product][populate][8]=api_data_list_page.apiData.apiRestDetail.specUrls&populate[product][populate][9]=api_data_list_page.apiData.apiSoapDetail.*&populate[product][populate][10]=guide_list_page&populate[product][populate][11]=tutorial_list_page&populate[seo][populate]=*,metaImage,metaSocial.image&pagination[pageSize]=1000&pagination[page]=1' |
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.
This is the updated query with use_case_list_page:
api/release-notes/?populate[bannerLinks][populate][0]=icon&populate[product][populate][0]=logo&populate[product][populate][1]=bannerLinks.icon&populate[product][populate][2]=overview&populate[product][populate][3]=quickstart_guide&populate[product][populate][4]=release_note&populate[product][populate][5]=api_data_list_page&populate[product][populate][6]=api_data_list_page.apiData.*&populate[product][populate][7]=api_data_list_page.apiData.apiRestDetail.slug&populate[product][populate][8]=api_data_list_page.apiData.apiRestDetail.specUrls&populate[product][populate][9]=api_data_list_page.apiData.apiSoapDetail.*&populate[product][populate][10]=guide_list_page&populate[product][populate][11]=tutorial_list_page&populate[product][populate][12]=use_case_list_page&populate[seo][populate]=*,metaImage,metaSocial.image&pagination[pageSize]=1000&pagination[page]=1
List of Changes
Optimize GitBook sync workflow to reduce API calls and improve performance
Motivation and Context
How Has This Been Tested?
cd packages/gitbook-docs
npm run compile
npm run sync-all-metadata
Screenshots (if appropriate):
Types of changes
Checklist: