Skip to content

Save a list of stopped services for mobile provider - #5822

Merged
Zensey merged 6 commits into
masterfrom
fix/mobile-prov-services-state
Jul 19, 2023
Merged

Save a list of stopped services for mobile provider#5822
Zensey merged 6 commits into
masterfrom
fix/mobile-prov-services-state

Conversation

@Zensey

@Zensey Zensey commented Jun 22, 2023

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread config/flags_service_start.go Outdated
// FlagStoppedServices a comma-separated list of stopped services.
FlagStoppedServices = cli.StringFlag{
Name: "stopped-services",
Usage: "Comma separated list of stopped services.",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need an extra flag for stopped services? All services without active services already list stopped services, right?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are tracking which services to start so to me it seems like:

all services - which to start = which are stopped

:)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag is used in order to store a list of stopped services in a config file (user-config.toml).
On mobile app shutdown we store this list, so that on app start - we can start exactly the same set of services.

@Zensey Zensey Jul 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soffokl
any objections ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I still see no reason for storing extra flags for stopped services if you can get them from other flags.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a flag:

	// FlagActiveServices a comma-separated list of active services.
	FlagActiveServices = cli.StringFlag{
		Name:  "active-services",
		Usage: "Comma separated list of active services.",
		Value: strings.Join([]string{"wireguard", "scraping", "data_transfer"}, ","),
	}

If you stop the app, you do not overwrite that flag. If you start the app these services are started automatically.
If you stop a single service from the UI it overwrites the flag to not start that service on the next restart.

@Zensey Zensey Jul 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soffokl

If you stop the app, you do not overwrite that flag.

In the mobile provider use-case when we need to stop the node temporarilty (shortage of battery or traffic) we don't stop the node, we just stop all active services.
Yes, this is a workaround, the reason is - if I stop/start the whole node a few times it misbehaves.

@soffokl soffokl Jul 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I stop/start the whole node a few times it misbehaves.

This sounds like we need to fix that problem instead of adding more flags/workarounds.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soffokl
Ok. I opened the bug: #5840

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soffokl
Yes, it's a workaround. Bug is not trivial to fix. May be we could use this work-around (only for mobile) until the bug is fixed ?

@Zensey
Zensey force-pushed the fix/mobile-prov-services-state branch from c635f06 to a61d527 Compare June 27, 2023 11:03
@Zensey
Zensey force-pushed the fix/mobile-prov-services-state branch from a61d527 to 959c7db Compare June 27, 2023 11:28
@codecov-commenter

codecov-commenter commented Jun 27, 2023

Copy link
Copy Markdown

Codecov Report

Merging #5822 (6f673df) into master (04b358b) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #5822      +/-   ##
==========================================
- Coverage   37.66%   37.58%   -0.08%     
==========================================
  Files         372      372              
  Lines       20721    20748      +27     
==========================================
- Hits         7804     7799       -5     
- Misses      12144    12172      +28     
- Partials      773      777       +4     
Impacted Files Coverage Δ
config/flags_service_start.go 0.00% <0.00%> (ø)
mobile/mysterium/config.go 0.00% <0.00%> (ø)
mobile/mysterium/entrypoint.go 0.00% <ø> (ø)
mobile/mysterium/mobile_provider.go 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

@Zensey
Zensey force-pushed the fix/mobile-prov-services-state branch from 66fd4de to d172e7e Compare June 27, 2023 16:33
@Zensey
Zensey requested a review from soffokl July 3, 2023 12:00
Comment thread config/flags_service_start.go Outdated
FlagStoppedServices = cli.StringFlag{
Name: "stopped-services",
Usage: "Comma separated list of stopped services.",
Value: strings.Join([]string{}, ","),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm giving up. Mark this flag at least as hidden.

Suggested change
Value: strings.Join([]string{}, ","),
Value: strings.Join([]string{}, ","),
Hidden: true,

@Zensey
Zensey force-pushed the fix/mobile-prov-services-state branch from 385b94c to 9a914fa Compare July 19, 2023 11:07
@Zensey
Zensey merged commit 711c6c4 into master Jul 19, 2023
@Zensey
Zensey deleted the fix/mobile-prov-services-state branch July 19, 2023 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants