Skip to content
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

Server v2 #167

Draft
wants to merge 153 commits into
base: cuttingedge
Choose a base branch
from
Draft

Server v2 #167

wants to merge 153 commits into from

Conversation

C9Glax
Copy link
Owner

@C9Glax C9Glax commented Apr 19, 2024

@db-2001
While going over your PR (related issue) I thought about some off the issues we have with the API.
#113 (comment)

make the /Jobs/Waiting smaller? It seems like it's sending 3.8MB of text every second, which is hell on my data usage when accessing remotely. Is there a need to sync the full metadata of all manga jobs every second, instead of on demand?

Resolves #74
Resolves #114
Could be their own Requests.

By returning a list of JobIds and crosschecking locally which jobs exist, this should be a lot easier on data.

Generally the API transmits a lot more data than it needs to, so streamlining requests (stripping data, splitting single requests into multiple smaller ones) we can react a lot better to changes.

So extending your effort, I propose that we also get going on a Version 2 of the API.
The old API would still be accessible (for now), but we could gradually shift requests to the new one.

I have started writing out the API-Documentation.
If you have any more requests you would like to see, then write them in this PR.

@C9Glax C9Glax added the enhancement New feature or request label Apr 19, 2024
@db-2001
Copy link
Collaborator

db-2001 commented Apr 19, 2024

Yeah this makes sense, let's make a branch of the current working code, like a V01.XX.XX and generate a docker image for that so people can force their server to run that code.

@db-2001
Copy link
Collaborator

db-2001 commented Apr 19, 2024

And you can just merge my current json API branch into this Server V2 branch if you want a head start. Once that's done we can close my branch and Implement json API as part of V2 and not have any in V1

@C9Glax
Copy link
Owner Author

C9Glax commented Apr 19, 2024

You want a hard cut-over? The way I wrote the current requests, we could still have both parts running, and be future-proof (v3,4,5...) too.
But yea I will merge your current branch into this one

@db-2001
Copy link
Collaborator

db-2001 commented Apr 19, 2024

I think something as big as an API changing structure would be enough to do a hard cut-over, it's your call in the end but I think V2 being the version with the big API change makes sense to me and then we just leave the old version alone, maybe patch it every now and then if we need to but otherwise leave it alone.

@db-2001
Copy link
Collaborator

db-2001 commented Apr 20, 2024

Also do you want to take the lead on the server side of V2 then? That'll let me focus on doing the frontend V2 implementing all these manga pop-up and search/add pop-up changes that everybody's been wanting. I can also then implement any other changes as a result of the server V2. What do you think?

@C9Glax
Copy link
Owner Author

C9Glax commented Apr 20, 2024

Sounds like plan!
So it would essentially be complete V2 front- and backend, then a hard cut-over would make a lot of sense.

@ale-ben
Copy link

ale-ben commented Dec 25, 2024

Hi 👋
Happy holidays!

Since I have some free time, is there a list of stuff that needs to be done to push forward either tranga or tranga-website? (Not promising anything but I'll try)

@db-2001
Copy link
Collaborator

db-2001 commented Dec 25, 2024

Hey @ale-ben, thanks for the offer! Happy Holidays! I think on the front end, the current pure JavaScript + CSS needs to be migrated over to the react + Typescript format.

@ale-ben
Copy link

ale-ben commented Dec 25, 2024

Thanks for the reply @db-2001!
As far as I can tell, the migration to react + ts has already begun in the vite-react-ts branch. Am I wrong?

What still needs to be done?

@ale-ben
Copy link

ale-ben commented Dec 26, 2024

Btw, if u want to add me on discord here's the link https://discord.gg/jS94ybbn

@C9Glax
Copy link
Owner Author

C9Glax commented Dec 26, 2024

Hi @ale-ben !
Yes the frontend rewrite has moved to vite-react-ts.
For the most part things kind of work.
The Backend is in pg-v2 and is a postgres-database rewrite with ASP.NET Endpoints.
If you build it, a swagger doc should be generated with the endpoint-names. Functionality is not really there yet (searching works), as Entity Framework Code-First and Lazyloading are a bit of a pain to get to work correctly, but that is currently the biggest front of my workload.

@ale-ben
Copy link

ale-ben commented Dec 26, 2024

Ok, thanks for the update!
I'll try cloning pg-v2 and see what happens.

Since v2 branch is pg-v2 I'll close #308

@C9Glax
Copy link
Owner Author

C9Glax commented Dec 26, 2024

I might have lied, it is postgres-server-v2 (a merge between both attempts) ;)
https://github.com/C9Glax/tranga/tree/postgres-Server-V2

@ale-ben
Copy link

ale-ben commented Jan 4, 2025

Ok, it took a while due various compilation problems but finally I got the API working!

The main problem now is that frontend refuses to communicate with the API. It keeps getting stuck here:
https://github.com/C9Glax/tranga-website/blob/0402f9e6d003601b0f6976b8638c390d38cb8e69/Website/App.tsx#L129

Frontend tries to call {apiUri}/v2/Ping but I can find no mention of this endpoint here...

Am I missing something or is this endpoint not implemented yet?

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 9, 2025

@ale-ben
Haven't updated the frontend in a minute, so that Endpoint doesn't exist yet.

@ale-ben
Copy link

ale-ben commented Jan 10, 2025

@C9Glax

I'm kind of stuck, I pulled the latest changes from postgres-Server-V2 and now nothing seems to work.

I am trying to run the project but I keep getting this error:

handled exception. System.InvalidOperationException: No suitable constructor was found for entity type 'Chapter'. The following constructors had parameters that could not be bound to properties of the entity type: 
    Cannot bind 'parentManga', 'chapterNumber' in 'Chapter(Manga parentManga, string url, ChapterNumber chapterNumber, int? volumeNumber, string title)'
    Cannot bind 'chapterNumber' in 'Chapter(string parentMangaId, string url, ChapterNumber chapterNumber, int? volumeNumber, string title)'
Note that only mapped properties can be bound to constructor parameters. Navigations to related entities, including references to owned types, cannot be bound.

I tried everything that comes to mind (except for moving my head back a couple of commits):

  • erased everything and cloned back from postgres-Server-V2
  • dropped the pgsql db tables to have a fresh start
  • stared intently at the code
  • tried talking it out with dave (my rubber duck)
  • (in progress) tried in a different machine

What am I missing?

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 10, 2025

You are missing my untested changes to Chapter.cs... So nothing really apart from my own stupidity.
Basically I tried implementing a new number-type for Chapters like "5.3.5", but haven't gotten it to work with the database yet.

Give my regards to Dave, he tried his best but could not fathom my incompetence :)
This is the commit btw that breaks it. (I think).

I am gonna very busy for the coming 2 months at least, so apart from fixing bugs on the current branch, I am not gonna really be able to make a lot of progress... So if you wanna go wild, now is the time 👍🏼

@ale-ben
Copy link

ale-ben commented Jan 11, 2025

Ok, thanks for the info, I'll see what I can do. I will open a PR if I accomplish something.
What was the idea behind using a custom structure to represent chapter number instead of a simple string?
With some simple comparison tricks you could obtain the same results...
Mind if I give a try to the string route?

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 11, 2025

Go right ahead!

@ale-ben
Copy link

ale-ben commented Jan 11, 2025

Screenshot 2025-01-11 at 19 46 31
There seems to be something very wrong with Jobs...
I'm trying DownloadSingleChapterJob. The job is started and set as running but never marked as completed, and every couple of seconds a new job is spawned. Neither Dave nor I can figure out why.

@C9Glax Do you have any Idea?

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 11, 2025

Just a quick hunch:
The spawning is happening because we get Job (-states) from the PGSQL context, which we need to updated, should we change anything in the Job-Object, which we are not at the moment. So Tranga.cs Line 79 List<Job> runJobs = context.Jobs.Where(j => j.state <= JobState.Running).ToList().Where(j => j.NextExecution < DateTime.UtcNow).ToList(); still returns the old state, which is kind of a pain tbh...

What probably is the best idea (threading, database updates, yadada), is to have a separate copy of the Jobs, that we then sync with the database after every check? That way changes to the Job-Objects can be made independent of the pgsql context and still be up-to-date enough for other applications?

@ale-ben
Copy link

ale-ben commented Jan 12, 2025

The solution appears to be quite simpler than what either of us expected (I feel like Dave knew though...)!
Preventing a thread to be spawned if a job with the same ID exists in RunningJobs apparently is enough to give the job time to sync the new state with pg.
This is the only thing I had to do: https://github.com/ale-ben/tranga/blob/dd1d67ac1190ae2c8af6e2537f672c271b819160/API/Tranga.cs#L85-L86

My formatter is quite aggressive and I haven't figured out how to tune it yet, sorry if some files are reformatted...

I opened a draft PR to track the changes between my branch and the official repo, you can follow my progresses here: #317

@ale-ben
Copy link

ale-ben commented Jan 12, 2025

Screenshot 2025-01-12 at 15 57 52

Unfortunately, with every solution comes a new problem...

I just noticed that creating identifiers using random numbers has a huge downside, if you search multiple times for the same manga (using the same connector), pg will have multiple entries (one for each search) for the same manga.
This could lead to incoherent results down the line...
As far as I know, this also extends to Chapters (multiple download all jobs generate multiple chapters for the same chapter).

I would suggest going back to using non-random based IDs like V1 used to do (for example Manga-[MangaConnectorId]-[ConnectorId]).
The only alternative I can think of would be before each operation (search manga, get chapters, ...) to verify if the entity exists in the context but it is a lot more expensive than just generating Ids based on something.
What do you think?

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 12, 2025

Nice!
On the formatter part: can you configure it to use strongly-typed Variable-Types? I really dislike the "var" stuff. It reallyesses with readability...

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 12, 2025

pg will have multiple entries (one for each search) for the same manga.

Yes, the primary Key should consist of ConnectorId and MangaConnectorId), or if we want to adhere to a standard-naming-scheme - which is why I introduced this anyways, a Hashing function taking those two...

@ale-ben
Copy link

ale-ben commented Jan 12, 2025

Nice, I was hoping for hashing IDs!
Should we keep the initial descriptor (Manga-, Chapter-, ...) and hash the last part or just drop the descriptor?
Will start on it immediately.

Formatter is fixed btw

@ale-ben
Copy link

ale-ben commented Jan 12, 2025

Also, sorry for the continuous pings...

@C9Glax
Copy link
Owner Author

C9Glax commented Jan 12, 2025

Should we keep the initial descriptor (Manga-, Chapter-, ...) and hash the last part or just drop the descriptor?

That's the idea 👍🏼

Don't worry about the pings, this is better than working on stuff for a few hours and then disagreeing on how to do something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment