Closed heldersepu closed 1 month ago
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! Btw Website Tribler.org has bleeding edge downloads on front-page.
some more logs that might help:
2024-04-28 13:41:59,598 - INFO - RESTComponent(174) - RESTComponent: waiting for KnowledgeComponent[0m
2024-04-28 13:41:59,598 - INFO - RESTComponent(174) - RESTComponent: waiting for TunnelsComponent[0m
2024-04-28 13:41:59,599 - INFO - RESTComponent(174) - RESTComponent: waiting for TorrentCheckerComponent[0m
2024-04-28 13:41:59,599 - INFO - RESTComponent(174) - RESTComponent: waiting for DatabaseComponent[0m
2024-04-28 13:41:59,601 - INFO - RESTComponent(50) - Adding: "/settings"...[0m
2024-04-28 13:41:59,602 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,602 - INFO - RESTComponent(50) - Adding: "/shutdown"...[0m
2024-04-28 13:41:59,603 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,603 - INFO - RESTComponent(50) - Adding: "/debug"...[0m
2024-04-28 13:41:59,605 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,605 - INFO - RESTComponent(50) - Adding: "/downloads"...[0m
2024-04-28 13:41:59,621 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,621 - INFO - RESTComponent(50) - Adding: "/createtorrent"...[0m
2024-04-28 13:41:59,622 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,622 - INFO - RESTComponent(50) - Adding: "/statistics"...[0m
2024-04-28 13:41:59,623 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,623 - INFO - RESTComponent(50) - Adding: "/libtorrent"...[0m
2024-04-28 13:41:59,624 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,625 - INFO - RESTComponent(50) - Adding: "/torrentinfo"...[0m
2024-04-28 13:41:59,626 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,627 - INFO - RESTComponent(50) - Adding: "/metadata"...[0m
2024-04-28 13:41:59,630 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,630 - INFO - RESTComponent(50) - Adding: "/search"...[0m
2024-04-28 13:41:59,631 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,631 - INFO - RESTComponent(50) - Adding: "/knowledge"...[0m
2024-04-28 13:41:59,635 - INFO - RESTComponent(58) - OK[0m
2024-04-28 13:41:59,725 - INFO - RESTManager(115) - Starting RESTManager...[0m
2024-04-28 13:41:59,727 - INFO - RESTManager(126) - Set security scheme and apply to all endpoints[0m
2024-04-28 13:41:59,727 - INFO - RESTManager(133) - Remove head[0m
2024-04-28 13:42:00,116 - INFO - RESTManager(140) - Http enabled[0m
2024-04-28 13:42:00,116 - INFO - RESTManager(154) - Starting HTTP REST API server on port 0...[0m
I'm looking at the code for the Starting HTTP REST API server on port 0
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 hardcoded that to:
http_enabled: bool = True
http_port: int = 43195
it was still showing the Starting HTTP REST API server on port 0
then I hardcoded it here: https://github.com/Tribler/tribler/blob/main/src/tribler/core/components/restapi/rest/rest_manager.py#L151 after that it seem to api is replying as it should on the hardcoded port
I think this is where the issue was introduced: https://github.com/Tribler/tribler/commit/74337b237951f0002c13f9550c7941db8366544f#diff-f4a4e039117c56099dbd14a77f0d9d3f8a65e165f556a9a9dbf8e3256ed8ce9bR147-R150
# The -1 value is assigned for backward compatibility reasons when the port is not specified.
# When RESTManager actually uses the value, it converts -1 to zero.
# It is possible that later we can directly store zero to config.api.http_port, but I prefer to be safe now.
config.api.http_port = api_port or -1
So we are overwriting the config port, why even have that in the config then?
@kozlovsky maybe you can help with this ...
Hi @heldersepu! Can you please tell me your scenario and why you want a dedicated HTTP port?
If you want to access Tribler REST API, you should specify it as a CORE_API_PORT
environment variable. It is specified in the "Tribler REST API" documentation:
Alternatively, requests can be made using Swagger UI by starting Tribler and opening
http://localhost:<port>/docs
in a browser. The port can be specified by setting up "CORE_API_PORT" environment variable.
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 http_port
value considered just as a recommendation that can be ignored by the Tribler Core process.
@kozlovsky Have you tried the api lately? As is it is unusable... For me the rest api just does not work on 7.14, i'm assuming that the port is the issue because when I hard code it works fine
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:
#!/bin/bash
## Script to test the api
IFS=$'\n'
key=$(cat ~/.Tribler/7.14/triblerd.conf | grep "key =")
key="${key/"key = "/""}"
port=$(cat ~/.Tribler/7.14/triblerd.conf | grep "port =")
port="${port/"port = "/""}"
curl -H "X-Api-Key: $key" -s http://localhost:$port/downloads
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 CORE_API_PORT
environment variable.
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:
Starting HTTP REST API server on port 0
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. The topic is a bit complicated and includes several independent sub-issues.
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 config.write()
.
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 (https://github.com/Tribler/tribler/issues/7736), queueing will become a priority.
once we have a webui (#7736), queueing will become a priority.
... but we already have a webui (swagger-ui) :face_with_head_bandage:
I guess that should come with "for advanced users only" disclaimer
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
config.write()
.
I just confirmed this...
added a self.logger.info(settings['api']['http_port'])
and the port in the logs is different than what we have in the triblerd.conf
now to hunt for where to add that config.write()
I think I smashed this bug take a look at that PR...
In the class RESTManager
we had:
if self.config.http_port != api_port:
self.config.http_port = api_port
and no writing the config after that, in part because it was not possible; That config was not a real config(TriblerConfig
) but of type APISettings
, just bad naming convention used in that variable that will confuse anyone, at least it confused me for a while; Now we have the "proper" config and writing after the port is changed
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 config.write()
after RESTManager.start
.
we disagree there ...
I firmly believe this is a bug! :bug:
Knowing that there is a TriblerConfig
, we just called a parameter of type APISettings
simply config
, 🤯 ... I mean seriously!?! one could argue we should minify all the code, if we do not care about naming variables something close to what they are, that will save a lot of characters on the code. Code golf anyone? :golf:
Now joking aside, there is no need for two params:
RESTManager(config=config.api, root_endpoint=self.root_endpoint, state_dir=config.state_dir)
when the same can be accomplished with one:
RESTManager(config=config, root_endpoint=self.root_endpoint)
and that way we pass a real config to the rest manager
Also inside the class RESTManager
we only invoke the write only when is necessary.
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 triblerd.conf
file for the following reasons:
An application configuration file should contain non-default option values that the application must use. If the config file contains the port value, then the application should use this port value only and report an error if another process occupies the port.
An application should not dynamically update its config file by writing some volatile values. If the application randomly chooses a free port and then dynamically updates its config file with the new port value, it looks inside out. Ideally, an application should not update its configuration file at all, except in two cases:
If another process or script wants to obtain the application's current parameters, it should not take them from the configuration file:
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:
http_port
and key
values specified in the config file;http_port
and key
values in the config file with new values that are ignored on the next run.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.
http_port
option value from the config file because currently (in Tribler 7.11 and above), it contains not a value set by a user but a randomly selected port value.http_port
config option after that, Tribler should use this port and not randomly select a free port.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, triblerd.info
, that contains all actual information about the current Tribler process. Then, we can add a new helper method, config.write_info()
, and use it instead of config.write()
whenever we want to update the information about the current process.
This approach has the following benefits:
triblerd.info
, so it should be easier to use.Before we implement it and release a new Tribler version with these changes, it is totally possible to use environment variables CORE_API_PORT
and CORE_API_KEY
to specify exact port/key values and use them for REST API. It should work for all recent Tribler versions, including Tribler 7.10 and 7.14. I understand that you prefer to read values from a file instead of setting values through environment variables; I'm just pointing out that this option works quite well right now.
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 triblerd.info
instead of triblerd.conf
.
@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 CORE_API_PORT
environment variable to the rescue... following that same pattern ... one file is not enough let's add a second, yeee.
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 CORE_API_PORT
then close tribler and open again I get an error:
RuntimeError: Tribler configuration conflicts with the current OS state: REST API port 52196 already in use
I never got that with v7.10
TLDR: Tribler should not write random temporary values to its own configuration file.
If I use CORE_API_PORT then close tribler and open again I get an error:
RuntimeError: Tribler configuration conflicts with the current OS state: REST API port 52196 already in use
For me, it works correctly:
[tribler-core PID:29468] 2024-05-10 22:51:06,503 - INFO - RESTManager(156) - Starting HTTP REST API server on port 33333...
...
[tribler-core PID:29468] 2024-05-10 22:51:06,654 - INFO - RESTManager(168) - HTTP REST API server started on port 33333
[tribler-core PID:29468] 2024-05-10 22:51:06,654 - INFO - RESTManager(149) - Swagger docs: http://127.0.0.1:33333/docs
[tribler-core PID:29468] 2024-05-10 22:51:06,655 - INFO - RESTManager(150) - Swagger JSON: http://127.0.0.1:33333/docs/swagger.json
By the way, your previous remark also does not correspond with what I see:
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:
Starting HTTP REST API server on port 0
This is what I see in logs when CORE_API_PORT
is not specified:
[tribler-core PID:9604] 2024-05-10 22:57:47,570 - INFO - RESTManager(156) - Starting HTTP REST API server on port 0...
...
[tribler-core PID:9604] 2024-05-10 22:57:47,705 - INFO - RESTManager(168) - HTTP REST API server started on port 65452
[tribler-core PID:9604] 2024-05-10 22:57:47,705 - INFO - RESTManager(149) - Swagger docs: http://127.0.0.1:65452/docs
[tribler-core PID:9604] 2024-05-10 22:57:47,706 - INFO - RESTManager(150) - Swagger JSON: http://127.0.0.1:65452/docs/swagger.json
TLDR: Tribler should not write random temporary values to its own configuration file.
But right now that is exactly what v7.14 is doing and at the wrong time
For me, it works correctly
the Tribler configuration conflicts with the current OS state
is happening all the time for me ... I'm on Ubuntu and i do
export CORE_API_PORT=52196; ~/Documents/GitHub/tribler/src/tribler.sh
If I wait a few minutes from the time I close to reopen it does work seems the port stays open for some reason
Thanks for the information; that's interesting...
By the way, your previous remark also does not correspond with what I see:
You are showing exactly the same line I did:
Starting HTTP REST API server on port 0
In my troubleshooting I stopped there, never saw others
I guess it was not really starting anything at that port just leftover junk messages
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 main
, this issue should now be resolved.
Looking at the recent changes to main I no longer see the src/tribler.sh
How do we open the UI now?
there used to be a src/README.md
that was remove too
It's easier to wait for an official 8.0
release build for your platform of choice. That said, if you really want to run from source right now:
You should build the UI (once) and run src/run_tribler.py
.
I do not recommend this, because we're still getting rid of the last traces of TriblerExperimental
in the source code. For example, your home dir is still set to ~/.TriblerExperimental
if you run the current version.
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 times