-
Notifications
You must be signed in to change notification settings - Fork 170
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
v0.19 upgrade #1277
v0.19 upgrade #1277
Conversation
My Api is getting pretty far too. Doing initial integration attempts. Not sure what the timeline is for v0.19. I probably could get it done in 2weeks. Really depends in what problems in run into. btw this has a good guide for publishing to maven |
Cool. Its tough to say yet when we'll release If you get your backwards-compatible API done soonish tho, we can use that one instead / in favor. |
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 was originally gonna just take over the changes from this PR into mine. But that no longer seems manageable, rebase it will be then not easy for sure.
I don't agree with some of the analysis changes
For the verification changes, the validateAuth, , success -> good, 4XX -> bad, 500 -> retry. So if validateAuth has the same behaviour is good. I have emulate validateAuth with that getMentionsCall for my API (for 0.18).
Also might have missed it but there should also be changes to the settings, the removal of that one. The postnotif setting do we completely remove it our just the UI parts. (Idc tbh)
Also you will have to redo the generate Types thing, there were some changes again |
K, addressed some of these above. Re-running |
BTW I also tested image uploading on voyager.lemmy.ml from this PR (I removed that cookie stuff), and it works. |
I was trying to test this PR but voyager seems to be having issues atm. I will test it later |
Alright it works but it seems to constantly display, "this account is being verified" on return out of a post. (It should not even be verifying the account) Using 0.18 accs seems to work better than I expected, I can browse but it does display the verification thingy stuck on jwt (prob cuz that doesn't fall back thus jwt verification fails as auth=null) (Maybe add a version warning? we can keep as is but we will definitely get issues around this) |
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 can't reproduce the previous "The account is being verified" issue anymore but there is definitely an edgecase somewhere where it doesn't properly purge the lock
Where would be the best place to add that toast? |
We had one in the root of our app by twiz, but that one is based of getSite which wouldn't work as getSite doesn't work across version. You could reuse it but add that nodeinfo stuff to get version but that's a lotta work. Or you could wait until my api is done.(merge as is, release in the meantime) Or we can use my LemmyApi.getVersion that does functionality but i would have to cut a prelease first. |
I'll wait until your API is done, I'll just merge this as is. Again, sry about the conflicts, lmk if there's anything I can do to help there. |
The hardest part that I still need to figure out, is that the API instance creation, now is suspended, (has to do a version call) meaning that it has todo that first before any other call, currently it doesn't adhere to that, which causes some race condition issues. As initial getSite / getPosts will done for the default. Currently the approaches that I can think of are:
|
(3) Makes the most sense to me also. Could be a part of the |
While fixing merge conflicts, I found some issues,
|
@MV-GH this is ready for you to look at.
To summarize my changes:
copy_generated_types
script.I tested most things, and they seem to be working correctly.
Limitations:
validate_auth
sections of the code: I'm not entirely sure how they should work, but that new endpoint is deployed on voyager.lemmy.ml .Next week we'll be testing some
0.19
on lemmy.ml to catch any last bugs, which will unfortunately probably break apps for a few days, before we do the0.19.0
lemmy backend release.I defer you to you on the best way to handle the upgrade to your new backwards-compatibility library / method. Even if we don't get it merged in time for this release, it'll still be great to have for future ones. I hope we don't get trapped in merge hell with our two PRs, but its not the end of the world if we do, we'll work through it. LMK anything I can do to help there.
cc @Nutomic