Fix Parse file renaming so tfss- files don't become legacy files #9
Conversation
Addresses issue mentioned in comments of PR parse-server-modules#7
|
I'm going for vacations, I've added @acinader as collaborator on the repo, so you can review / merge the PR's without me :) Keep it up guys! You are awesome |
|
Thanks @flovilmart! Have a nice vacation 🌞 @acinader I know you're off the rest of today, so feel free to review/merge this PR whenever you have a moment. |
|
I'm getting ready to run it. my one bit of feedback would be to make the newPrefix have some definite pattern to it, in case we need to pick them out with a regex to re-process if we discover a problem a few weeks down the road? |
|
@acinader Great idea! Check out the latest commit on this PR and let me know what you think. If it's all good, go ahead and click the "merge pull request" button. |
Current coverage is 16.87% (diff: 14.28%)
|
|
it looks good. figured i would test it first. should just be an hour or two. |
|
I was thinking about it in the last days: keeping the This way, both Parse Server and hosted Parse will give a URL that works: the first, using Parse's S3, and the latter using your own S3. @acinader @JeremyPlease don't you think that we could add the "File Renaming" as an option also? Meaning, if the user wants, he can disable it. |
|
duh. I like it waaaaay better and I suggest just remove file renaming entirely, even as an option. I can't really imagine the use case and then no data changes. migration becomes: run the script. verify it with a non prod parse-server. remove file key. if something goes south, just put key back. i'm currently in the final minutes of restoring a db to test against, but will now wait for @JeremyPlease to weigh in. If he concurs, I'll just wait to test against your idea. |
|
one other nit would be to add to the readme that it only works on ~1,000 classes? |
|
Exactly! I agree, and that is exactly the same way I believe it should be. |
|
@natanrolnik can you humor me and try and think of a use case where someone would want to rename? If we can't think of a potential use case, i'd suggest that changing data is dangerous enough to leave the option out unless there's an reason we can think of to include. Another advantage is to not have one more file naming scheme to worry about in the future.... |
|
@natanrolnik Nice find! I missed that PR/feature in Parse Server. @acinader I mostly agree. However, I think renaming should still be an option. Mainly so that clients using Parse Server can still see files uploaded by clients using api.parse.com. Example:
|
@acinader What do you mean? Is it even possible to have 1,000 classes in Parse or did you not mean "classes"? |
|
re: 1,000 classes: I did mean classes. there's a limit of 1,000 on the query to schema, so its a known limitation. clearly not a big deal in any event. re: file renaming. yup, good one. and i buy it. goes right to the heart of the one remaining problem with the parse migration: inability to support parse.com and parse-server simultaneously, which is giving me such heartburn :). but other uses may have different use cases where it is fine to have both new and old, so i def buy leaving it in as an option. The readme is going to need some explanation of the scenarios? which i think would be really helpful to users like me who are trying to figure out what is going on ;) |
|
i can test both today.... |
|
I agree @acinader . And it's such a shame that we can define a Mongo URI in the Parse.com dashboard, but we can't define a S3 bucket 😞 |
|
Right? ;) |
|
though it'd have to be bucket, prefix, two keys, and then what about google....i get it. though the transition to parse-server would really be seamless to end users and product managers if it weren't for that one thing. /rant |
|
Well, if not an S3 bucket, then a base URL. |
|
@JeremyPlease fyi, i think i'm waiting on you to weigh in and/or add a commit to start testing. lmk if that's not right. no rush, i got plenty to do here..... |
|
i'm running test now. |
|
Hey all. (sorry for delayed response) It seems like the most flexible solution would be to have a setting to enable/disable file renaming. Is that what you think too, @acinader? The tough part with all of this is going to be providing a good guide/documentation with the tradeoffs of each setup. |
|
Hi @JeremyPlease - yes, I'm pretty sure that that is the right option, and I agree that documenting it is tough as I'm having a hard time wrapping my pea brain around it. I'm running this pr now in any event and I'll report back once i've tested in an hour or so. |
|
looks good. as soon as i am done with the rename options that is running now, i'll restore my db and run the no rename options. |
|
Thanks @acinader. I'm going to open a separate issue to discuss the guide/documentation/options. Will link here after I gather my thoughts. |
|
@JeremyPlease I was talking right now to @acinader regarding adding the "no rename" option, but you were faster! Thanks! |
|
@acinader @natanrolnik Could use your input on issue #10. Thanks! |
|
no rename worked too. nice and thanks @JeremyPlease |
|
re: 1,000 classes: I did mean classes. there's a limit of 1,000 on the query to schema, so its a known limitation. clearly not a big deal in any event. It needs to run multiple queries using the "skip" query command somehow otherwise it only returns the first 1000 rows worth of data. |
Addresses issue mentioned in comments of PR #7