Skip to content

feat: rebase migrations, remove draft-js enitrely #10640

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

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

mattkrick
Copy link
Member

@mattkrick mattkrick commented Jan 8, 2025

Description

It's expected for the Migration CI test to fail. We're deleting them all!

fixes #10633

Rebases migrations since the current migrations rely on draft-js & we need to get rid of that package in order to upgrade our yarn version.

This also fixes a bug where the CDN_BASE_URL was not respected when seeding MeetingTemplate.illustrationUrl on an empty database. Proof of the fix below

image

Signed-off-by: Matt Krick <matt.krick@gmail.com>
Signed-off-by: Matt Krick <matt.krick@gmail.com>
@github-actions github-actions bot added the size/m label Jan 8, 2025
Signed-off-by: Matt Krick <matt.krick@gmail.com>
@github-actions github-actions bot added size/xl and removed size/m labels Jan 8, 2025
Copy link
Contributor

github-actions bot commented Jan 8, 2025

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

Signed-off-by: Matt Krick <matt.krick@gmail.com>
@mattkrick mattkrick changed the title chore: remove draft-js from migrations feat: rebase migrations, remove draft-js enitrely Jan 8, 2025
@mattkrick mattkrick marked this pull request as ready for review January 8, 2025 22:15
@github-actions github-actions bot requested a review from tianrunhe January 8, 2025 22:16
Copy link
Contributor

github-actions bot commented Jan 8, 2025

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size.

@mattkrick mattkrick requested review from rafaelromcar-parabol and removed request for tianrunhe January 8, 2025 22:16
Copy link
Contributor

@rafaelromcar-parabol rafaelromcar-parabol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • action-files.parabol.co is hardcoded multiple times and it shouldn't. Three in the new init migration and also six times in package/client/hooks/useMockPeekers.ts and twice in packages/client/types/constEnums.ts. Our storage could change of domain at any time and for private instances (PPMIs, PubSec or self-hosted) they should not use action-files.parabol.co for anything at all. Each instance should use the same file store provider and domain they are setting in their configuration and only that.
  • For the new migration, you are basically inserting using the self-hosted based path and then modifying it if there is FILE_STORE_PROVIDER that's not local. Simple and effective enough 😸 That LGTM.

I'm going to test this in an on-demand-env

@rafaelromcar-parabol
Copy link
Contributor

In my on-demand-env, I'm using AWS S3 as file store provider and when I started the environment for the first time, pre-deploy was executed with the following logs:

ServerID 145                                                                                                                                                                                                       
{"time":"2025-01-09T13:35:08.083Z","level":"info","message":"🚀 Predeploy Started v8.16.0 sha:c32591230eec6a125cebc1fa592158045c70a79f []"}                                                                        
{"time":"2025-01-09T13:35:08.087Z","level":"info","message":"🐘 Postgres Migration Started []"}                                                                                                                    
{"time":"2025-01-09T13:35:08.087Z","level":"info","message":"🔩 Postgres Extension Checks Started []"}                                                                                                             
{"time":"2025-01-09T13:35:08.087Z","level":"info","message":"   pgvector []"}                                                                                                                                      
{"time":"2025-01-09T13:35:08.192Z","level":"info","message":"🔩 Postgres Extension Checks Completed []"}                                                                                                           
{"time":"2025-01-09T13:35:09.984Z","level":"info","message":"  ✅ Migration: 2025-01-08T00:27:52.203Z_init []"}                                                                                                    
{"time":"2025-01-09T13:35:09.985Z","level":"info","message":"🐘 Postgres Migration Complete []"}                                                                                                                   
{"time":"2025-01-09T13:35:09.986Z","level":"info","message":"🔗 QueryMap Persistence Started []"}                                                                                                                  
{"time":"2025-01-09T13:35:09.992Z","level":"info","message":"⛓️ Prime Integrationgs Started []"}                                                                                                                    
{"time":"2025-01-09T13:35:09.994Z","level":"info","message":"⛅️ Push to CDN Started []"}                                                                                                                           
{"time":"2025-01-09T13:35:10.168Z","level":"info","message":"🔗 QueryMap Persistence Complete: 271 records added []"}                                                                                              
{"time":"2025-01-09T13:35:10.482Z","level":"info","message":"⛓️ Prime Integrations Complete []"}                                                                                                                    
{"time":"2025-01-09T13:35:12.225Z","level":"info","message":"⛅️ Uploaded 223 client assets to CDN []"}                                                                                                             
{"time":"2025-01-09T13:35:12.400Z","level":"info","message":"⛅️ Server upload complete. Pushed 0 assets to CDN []"}                                                                                                
{"time":"2025-01-09T13:35:12.400Z","level":"info","message":"⛅️ Push to CDN Complete []"}                                                                                                                          
{"time":"2025-01-09T13:35:12.401Z","level":"info","message":"🚀 Predeploy Complete []"}                                                                                                                            
Stream closed EOF for od-remove-draft-js/parabol-predeploy-sxlzj (parabol-predeploy)

All files were uploaded to action-files.parabol.co/on-demand-envs/build but nothing to the folder store. This is a fresh environment. I tried to manually run pushToCDN.js and it told me the following:

{"time":"2025-01-09T17:57:49.875Z","level":"info","message":"⛅️ Push to CDN Started []"}
{"time":"2025-01-09T17:57:52.267Z","level":"info","message":"⛅️ Uploaded 223 client assets to CDN []"}
{"time":"2025-01-09T17:57:52.570Z","level":"info","message":"⛅️ Server upload complete. Pushed 0 assets to CDN []"}
{"time":"2025-01-09T17:57:52.571Z","level":"info","message":"⛅️ Push to CDN Complete []"}

AWS vars have the correct value, as if they hadn't no files would be uploaded to the folder build. There are no template images in the bucket

@rafaelromcar-parabol
Copy link
Contributor

The problem with the files not being uploaded is being fixed in #10661

@mattkrick mattkrick merged commit db4832f into master Jan 10, 2025
12 of 13 checks passed
@mattkrick mattkrick deleted the chore/remove-draft-js branch January 10, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static assets are hardcoded to be readed from /self-hosted/ folder on the init migration
2 participants