-
Notifications
You must be signed in to change notification settings - Fork 452
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
After upgrade to 7.14.0 http_port keeps changing and api does not work #8007
Comments
Actually it looks like the port is now changing every time that tribler closes |
Wow, thank you for finding this weird bug! Btw could you possibly test if the experimental version on our front-page has for you the same weird bug? |
@synctext sorry I do not see any link to the experimental version |
Very helpful! |
some more logs that might help:
I'm looking at the code for the |
Just to test I changed a couple of things: https://github.com/Tribler/tribler/blob/main/src/tribler/core/components/restapi/rest/settings.py#L10-L11
it was still showing the then I hardcoded it here: I think this is where the issue was introduced:
So we are overwriting the config port, why even have that in the config then? |
@kozlovsky maybe you can help with this ... |
Hi @heldersepu! If you want to access Tribler REST API, you should specify it as a
The config option "http_port" is currently not intended for direct use. Historically, some fields of Tribler config, like "http_port," were used to communicate between GUI and Core Tribler processes: The GUI process set the values of these options and expected The Core process to read them. The original logic was that Tribler finds a free port and sticks to it by saving the found port value to settings, so after the relaunch, it uses the same port value. Unfortunately, it does not always work because another application can occupy this port, so when Tribler starts the next time, it cannot use the port value specified in the settings. Usually, for Tribler, it is not necessary to use a specific port value, and we do not want Tribler to stop with an error if the previously used port is unavailable, so, for a long time, the specified |
@kozlovsky Have you tried the api lately? As is it is unusable... The suggestion to use CORE_API_PORT environment variable, I see no reason for that, why can we not read the port from the tribbler config and use that? for example I expected this to work:
that was working fine on 7.10, no longer working on 7.14 |
... and for the record i'm all for Tribler not using a specific port value, yes if another application can occupy this port, we are in troubles, same applies to the But whatever port is used it must be "published" in the config and as far as I can see that is not happening, all I see in the logs is: |
Let me add some more context: The reason I started using the Rest API is because queueing is not an option in Tribler, so the only way I found to enforce a max simultaneous download is with a small script to check how many in progress and stop & start items to keep a max... Maybe I should work on incorporating that queue in the core instead of the workaround via api |
Hi @heldersepu, I apologize for the delayed response. I hope to answer in a few hours. |
@kozlovsky absolutely no rush |
Seems like Tribler writes a newly acquired port number to disk only on shutdown (when it's no longer relevant). A reasonable fix seems to be writing to disk just after the API server has started. That's just a matter of calling As far as queueing is concerned, I completely agree that we need it. Right now if you have 100 downloads running, it will actually start all 100 swarms. It's much better to let libtorrent deal with starting/stopping swarms. In the past we avoided auto-management, because of the GUI work required. Our anonymous downloading stack also complicates matters. In the future, once we have a webui (#7736), queueing will become a priority. |
... but we already have a webui (swagger-ui) 🤕 |
I just confirmed this... now to hunt for where to add that |
I think I smashed this bug take a look at that PR...
and no writing the config after that, in part because it was not possible; That config was not a real config( |
I don't really think that's a bug, it's just how we pass parts of the main config to the components. Considering you can't write those to disk, I would just call |
we disagree there ... I firmly believe this is a bug! 🐛 Now joking aside, there is no need for two params: Also inside the |
Hi @heldersepu! I understand your need to obtain the actual port value. However, I'm afraid I have to disagree that the Tribler core process should save the current port value to the
Currently, the Tribler Core process indeed writes a port value to the config file. But this behavior looks like a bug we should fix, not the one we should use for inter-process communication. Specifically, the following looks like an incorrect behavior:
As I understand your current opinion, you say Tribler Core updates the config file too late (on process exit), and should do it earlier. I believe it shouldn't update the config file at all. So, I think we can do the following to fix Tribler's current incorrect behavior.
But now, what should we do with your original problem? How should we make the current port/key values available to other applications? The simplest thing we can do is write them into a file, but it should not be a config file. We can create a second file, This approach has the following benefits:
Before we implement it and release a new Tribler version with these changes, it is totally possible to use environment variables Regarding your PR #8022, I think we can use it as a base for these refactoring; I just believe we should write to a different file like |
@kozlovsky I feel you are over engineering a solution here ... Tribler already had a way to "set" a port via the conf file, but one way is not enough so At this point I honestly could care less how you do it or how long it takes to release it, I was ready to roll back to v7.10 ... but I spent the time and got my own fork working & will run from there for the foreseeable future One more thing you might want to add to this bug, if I use |
TLDR: Tribler should not write random temporary values to its own configuration file.
For me, it works correctly:
By the way, your previous remark also does not correspond with what I see:
This is what I see in logs when
|
But right now that is exactly what v7.14 is doing and at the wrong time |
the |
Thanks for the information; that's interesting... |
You are showing exactly the same line I did: |
I upgraded and both http_port and key were removed from the config. I see here CORE_API_PORT and CORE_API_KEY should be set in the env. Maybe changes like thing like this should be mentioned in stderr instead of the current spam, or sdtout which is shorter. ... I think env is the wrong way to pass config because if running as a service it means editing tribler.sh instead of triblerd.conf which is clearly a failure to separate config from code. Using a config file and having a getCurrentConfig feature for the app is the best way to implement (eg postconf from postfix, SHOW ALL from postgres, etc). |
After the recent changes to |
Looking at the recent changes to main I no longer see the there used to be a |
It's easier to wait for an official You should build the UI (once) and run I do not recommend this, because we're still getting rid of the last traces of |
Describe the bug
I expected my settings to be maintained but some how the
http_port
changedI had:
after upgrade
I just tested running from source:
here is the output of the
triblerd.conf
after opening an closing a couple of timesThe text was updated successfully, but these errors were encountered: