add nix-collect-garbage endpoint #21

Closed
def wants to merge 70 commits from def/nix-collect-garbage-endpoint into master
7 changed files with 401 additions and 2 deletions

0
.gitignore vendored Executable file → Normal file
View File

View File

@ -8,9 +8,12 @@ from selfprivacy_api.graphql.mutations.mutation_interface import (
GenericJobMutationReturn,
GenericMutationReturn,
MutationReturnInterface,
GenericJobMutationReturn,
)
import selfprivacy_api.actions.system as system_actions
from selfprivacy_api.graphql.common_types.jobs import job_to_api_job
from selfprivacy_api.jobs.nix_collect_garbage import start_nix_collect_garbage
import selfprivacy_api.actions.ssh as ssh_actions
@ -195,3 +198,14 @@ class SystemMutations:
message=f"Failed to pull repository changes:\n{result.data}",
code=500,
)
@strawberry.mutation(permission_classes=[IsAuthenticated])
def nix_collect_garbage(self) -> GenericJobMutationReturn:
job = start_nix_collect_garbage()

add a handler for an error too

add a handler for an error too
return GenericJobMutationReturn(
success=True,
code=200,
message="Garbage collector started...",
job=job_to_api_job(job),
)

View File

@ -67,8 +67,8 @@ def move_folder(
try:
data_path.mkdir(mode=0o750, parents=True, exist_ok=True)
except Exception as e:
print(f"Error creating data path: {e}")
except Exception as error:
print(f"Error creating data path: {error}")
return
try:

View File

@ -0,0 +1,154 @@
import re
import subprocess
from typing import Tuple, Iterable
from selfprivacy_api.utils.huey import huey
from selfprivacy_api.jobs import JobStatus, Jobs, Job
def marked this conversation as resolved Outdated

typo

typo
class ShellException(Exception):
inex marked this conversation as resolved Outdated

So the user sees "We are sorry, result was not found :(" after starting garbage collection. What would this even mean?

So the user sees "We are sorry, result was not found :(" after starting garbage collection. What would this even mean?

done

done

Remove emotions from the message.

Remove emotions from the message.

done

done
"""Shell-related errors"""
COMPLETED_WITH_ERROR = "Error occurred, please report this to the support chat."
RESULT_WAS_NOT_FOUND_ERROR = (
"We are sorry, garbage collection result was not found. "
"Something went wrong, please report this to the support chat."
inex marked this conversation as resolved Outdated

Variable PIPE not allowed in type expression

Variable `PIPE` not allowed in type expression

done

done
)
CLEAR_COMPLETED = "Garbage collection completed."
def delete_old_gens_and_return_dead_report() -> str:
inex marked this conversation as resolved Outdated

Add typing to functions

Add typing to functions

done

done
subprocess.run(
["nix-env", "-p", "/nix/var/nix/profiles/system", "--delete-generations old"],
check=False,
)
result = subprocess.check_output(["nix-store", "--gc", "--print-dead"]).decode(
inex marked this conversation as resolved Outdated

You mean some very specific type of line here. Please, paste an example as a docstring and maybe rename function for clarity

You mean some very specific type of line here. Please, paste an example as a docstring and maybe rename function for clarity

done

done

Expression of type "IO[bytes] | None" cannot be assigned to return type "Popen[Unknown]"
Type "IO[bytes] | None" cannot be assigned to type "Popen[Unknown]"
"IO[bytes]" is incompatible with "Popen[Unknown]"

Expression of type "IO[bytes] | None" cannot be assigned to return type "Popen[Unknown]" Type "IO[bytes] | None" cannot be assigned to type "Popen[Unknown]" "IO[bytes]" is incompatible with "Popen[Unknown]"

done

done
"utf-8"
)
return " " if result is None else result
inex marked this conversation as resolved Outdated

There is JobStatus.Error, please use it

There is `JobStatus.Error`, please use it

Also, the whole tuple doesn't look very good. Why do you return the tuple[Literal[JobStatus.FINISHED], Literal[100], Literal['Completed with an error'], Literal['We are sorry, result was not found :(']] | tuple[Literal[JobStatus.FINISHED], Literal[100], Literal['Cleaning completed.'], str] type?

Also, the whole tuple doesn't look very good. Why do you return the `tuple[Literal[JobStatus.FINISHED], Literal[100], Literal['Completed with an error'], Literal['We are sorry, result was not found :(']] | tuple[Literal[JobStatus.FINISHED], Literal[100], Literal['Cleaning completed.'], str]` type?

(1) done

(1) done
def run_nix_collect_garbage() -> Iterable[bytes]:
process = subprocess.Popen(
["nix-store", "--gc"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT
)
return process.stdout if process.stdout else iter([])
def parse_line(job: Job, line: str) -> Job:
"""
inex marked this conversation as resolved Outdated

In current version of jobs, it is not necessary to set progress to 100 explicitly when finishing the job

In current version of jobs, it is not necessary to set progress to 100 explicitly when finishing the job

Okay, but I won't change it so I don't have to rewrite the argument return.

Okay, but I won't change it so I don't have to rewrite the argument return.

you should rewrite the return type of this function

you should rewrite the return type of this function

done

done
We parse the string for the presence of a final line,
with the final amount of space cleared.
Simply put, we're just looking for a similar string:

You can pass job as an argument to a function to not have these complex resturns

You can pass job as an argument to a function to not have these complex resturns

Yeah, but I've already done it my way

Yeah, but I've already done it my way

you didn't

you didn't
"1537 store paths deleted, 339.84 MiB freed".
"""
pattern = re.compile(r"[+-]?\d+\.\d+ \w+(?= freed)")
inex marked this conversation as resolved Outdated

Consider renaming to process_stream.
We do not stream processes here, we process streams.

Consider renaming to `process_stream`. We do not stream processes here, we process streams.

done

done

Errored jobs are currently not handled by this endpoint.
Add a handler and a test where the call errors out.

Errored jobs are currently not handled by this endpoint. Add a handler and a test where the call errors out.
match = re.search(pattern, line)
if match is None:
raise ShellException("nix returned gibberish output")
else:
Jobs.update(
job=job,
status=JobStatus.FINISHED,
status_text=CLEAR_COMPLETED,
result=f"{match.group(0)} have been cleared",
)
inex marked this conversation as resolved Outdated

no typing

no typing

done

done
return job
def process_stream(job: Job, stream: Iterable[bytes], total_dead_packages: int) -> None:
completed_packages = 0
prev_progress = 0
for line in stream:
line = line.decode("utf-8")
def marked this conversation as resolved Outdated

This may lead to percents not summing up to 100. You better count the number of paths deleted and divide them by a total.

This may lead to percents not summing up to 100. You better count the number of paths deleted and divide them by a total.
if "deleting '/nix/store/" in line:
completed_packages += 1
percent = int((completed_packages / total_dead_packages) * 100)
if percent - prev_progress >= 5:
Jobs.update(
job=job,
status=JobStatus.RUNNING,
progress=percent,
status_text="Cleaning...",
)
prev_progress = percent
elif "store paths deleted," in line:
parse_line(job, line)
def get_dead_packages(output) -> Tuple[int, float]:
dead = len(re.findall("/nix/store/", output))
percent = 0
if dead != 0:
inex marked this conversation as resolved Outdated

no typing

no typing

done

done
percent = 100 / dead
return dead, percent
@huey.task()
def calculate_and_clear_dead_paths(job: Job):
Jobs.update(
job=job,
status=JobStatus.RUNNING,
progress=0,
status_text="Calculate the number of dead packages...",
inex marked this conversation as resolved Outdated

You've declared that job is of Jobs type, but this function expects the Job type.

You've declared that `job` is of `Jobs` type, but this function expects the `Job` type.

done

done

You've broken it even further. Also improve your tests to check the status of Job object.

You've broken it even further. Also improve your tests to check the status of Job object.

the error highlighting tells me that I've definitely fixed it now.

the error highlighting tells me that I've definitely fixed it now.

done

done
)
dead_packages, package_equal_to_percent = get_dead_packages(
delete_old_gens_and_return_dead_report()
)
if dead_packages == 0:
Jobs.update(
job=job,
status=JobStatus.FINISHED,
status_text="Nothing to clear",
result="System is clear",
)
return True
Jobs.update(
job=job,
status=JobStatus.RUNNING,
progress=0,
status_text=f"Found {dead_packages} packages to remove!",
)
stream = run_nix_collect_garbage()
try:
process_stream(job, stream, dead_packages)
except ShellException as error:
Jobs.update(
job=job,
status=JobStatus.ERROR,
status_text=COMPLETED_WITH_ERROR,
error=RESULT_WAS_NOT_FOUND_ERROR,
)
def start_nix_collect_garbage() -> Job:
job = Jobs.add(
type_id="maintenance.collect_nix_garbage",
name="Collect garbage",
description="Cleaning up unused packages",
)
calculate_and_clear_dead_paths(job=job)
Jobs.update(
job=job,
status=JobStatus.FINISHED,
status_text="Collect garbage finished",
progress=100,
)
return job

View File

@ -7,6 +7,8 @@ from selfprivacy_api.services.tasks import move_service
from selfprivacy_api.jobs.upgrade_system import rebuild_system_task
from selfprivacy_api.jobs.test import test_job
from selfprivacy_api.jobs.nix_collect_garbage import calculate_and_clear_dead_paths
if environ.get("TEST_MODE"):
from tests.test_huey import sum

0
setup.py Executable file → Normal file
View File

View File

@ -0,0 +1,229 @@
# pylint: disable=redefined-outer-name
# pylint: disable=unused-argument
# pylint: disable=missing-function-docstring
import pytest
from selfprivacy_api.utils.huey import huey
from selfprivacy_api.jobs import JobStatus, Jobs
from tests.test_graphql.common import (
get_data,
assert_ok,
assert_empty,
)
from selfprivacy_api.jobs.nix_collect_garbage import (
get_dead_packages,
parse_line,
ShellException,
)
OUTPUT_PRINT_DEAD = """
inex marked this conversation as resolved Outdated
  • Do you actually need this dependency?
  • This breaks CI. New deps must be discussed.
- Do you actually need this dependency? - This breaks CI. New deps must be discussed.

done

done

deadcode

deadcode
finding garbage collector roots...
determining live/dead paths...
/nix/store/02k8pmw00p7p7mf2dg3n057771w7liia-python3.10-cchardet-2.1.7
/nix/store/03vc6dznx8njbvyd3gfhfa4n5j4lvhbl-python3.10-async-timeout-4.0.2
/nix/store/03ybv2dvfk7c3cpb527y5kzf6i35ch41-python3.10-pycparser-2.21
/nix/store/04dn9slfqwhqisn1j3jv531lms9w5wlj-python3.10-hypothesis-6.50.1.drv
/nix/store/04hhx2z1iyi3b48hxykiw1g03lp46jk7-python-remove-bin-bytecode-hook
"""
OUTPUT_COLLECT_GARBAGE = """
removing old generations of profile /nix/var/nix/profiles/per-user/def/channels
finding garbage collector roots...
deleting garbage...
deleting '/nix/store/02k8pmw00p7p7mf2dg3n057771w7liia-python3.10-cchardet-2.1.7'
deleting '/nix/store/03vc6dznx8njbvyd3gfhfa4n5j4lvhbl-python3.10-async-timeout-4.0.2'
deleting '/nix/store/03ybv2dvfk7c3cpb527y5kzf6i35ch41-python3.10-pycparser-2.21'
deleting '/nix/store/04dn9slfqwhqisn1j3jv531lms9w5wlj-python3.10-hypothesis-6.50.1.drv'
deleting '/nix/store/04hhx2z1iyi3b48hxykiw1g03lp46jk7-python-remove-bin-bytecode-hook'
deleting unused links...
note: currently hard linking saves -0.00 MiB
190 store paths deleted, 425.51 MiB freed
"""
OUTPUT_COLLECT_GARBAGE_ZERO_TRASH = """
removing old generations of profile /nix/var/nix/profiles/per-user/def/profile
removing old generations of profile /nix/var/nix/profiles/per-user/def/channels
finding garbage collector roots...
deleting garbage...
deleting unused links...
note: currently hard linking saves 0.00 MiB
0 store paths deleted, 0.00 MiB freed
"""
# ---

I would suggest mocking system calls, and not your own functions.

I would suggest mocking system calls, and not your own functions.
def test_parse_line():
txt = "note: currently hard linking saves -0.00 MiB 190 store paths deleted, 425.51 MiB freed"
job = Jobs.add(
name="name",
type_id="parse_line",
description="description",
)
output = parse_line(job, txt)
assert output.result == "425.51 MiB have been cleared"
assert output.status == JobStatus.FINISHED
assert output.error is None
inex marked this conversation as resolved Outdated

what are you trying to do here exactly?(dead code?)

what are you trying to do here exactly?(dead code?)

done

done
def test_parse_line_with_blank_line():
txt = ""
job = Jobs.add(
name="name",
type_id="parse_line",
description="description",
)
with pytest.raises(ShellException):
output = parse_line(job, txt)
def test_get_dead_packages():
assert get_dead_packages(OUTPUT_PRINT_DEAD) == (5, 20.0)
def test_get_dead_packages_zero():
assert get_dead_packages("") == (0, 0)
RUN_NIX_COLLECT_GARBAGE_MUTATION = """
mutation CollectGarbage {
system {
nixCollectGarbage {
success
message
code
job {
uid,
typeId,
name,
description,
status,
statusText,
progress,
createdAt,
updatedAt,
finishedAt,
error,
result,
}
}
}
}
"""
def test_graphql_nix_collect_garbage(authorized_client, fp):
assert huey.immediate is True
fp.register(
["nix-env", "-p", "/nix/var/nix/profiles/system", "--delete-generations old"],
stdout="",

Lacking test of trying to call mutation without auth

Lacking test of trying to call mutation without auth

done

done
)
fp.register(["nix-store", "--gc", "--print-dead"], stdout=OUTPUT_PRINT_DEAD)
fp.register(["nix-store", "--gc"], stdout=OUTPUT_COLLECT_GARBAGE)

This test just hangs and doesn't finish

This test just hangs and doesn't finish

done

done
response = authorized_client.post(
inex marked this conversation as resolved Outdated

This is not a query, this is a Mutation. Both semantically and you wrote it yourself.

This is not a query, this is a Mutation. Both semantically and you wrote it yourself.

If you mean I don't ask for mutation fields, I've changed that.

so, done?

If you mean I don't ask for mutation fields, I've changed that. so, done?

no, it is about naming.

no, it is about naming.

done

done
"/graphql",
json={
"query": RUN_NIX_COLLECT_GARBAGE_MUTATION,
},
)
output = get_data(response)["system"]["nixCollectGarbage"]
assert_ok(output)
inex marked this conversation as resolved Outdated

is True already checks that it is a boolean and it is True

`is True` already checks that it is a boolean and it is True

done

done
assert output["job"] is not None
assert output["job"]["status"] == "FINISHED"
assert output["job"]["error"] is None
assert (
fp.call_count(
[
"nix-env",
"-p",
"/nix/var/nix/profiles/system",
"--delete-generations old",
]
)
== 1
)
assert fp.call_count(["nix-store", "--gc", "--print-dead"]) == 1
assert fp.call_count(["nix-store", "--gc"]) == 1
def test_graphql_nix_collect_garbage_return_zero_trash(authorized_client, fp):
assert huey.immediate is True
fp.register(
["nix-env", "-p", "/nix/var/nix/profiles/system", "--delete-generations old"],
stdout="",
)
fp.register(["nix-store", "--gc", "--print-dead"], stdout=OUTPUT_PRINT_DEAD)
fp.register(["nix-store", "--gc"], stdout=OUTPUT_COLLECT_GARBAGE_ZERO_TRASH)
response = authorized_client.post(
"/graphql",
json={
"query": RUN_NIX_COLLECT_GARBAGE_MUTATION,
},
)
output = get_data(response)["system"]["nixCollectGarbage"]
assert_ok(output)
assert output["job"] is not None
assert output["job"]["status"] == "FINISHED"
assert output["job"]["error"] is None
assert (
fp.call_count(
[
"nix-env",
"-p",
"/nix/var/nix/profiles/system",
"--delete-generations old",
]
)
== 1
)
assert fp.call_count(["nix-store", "--gc", "--print-dead"]) == 1
def marked this conversation as resolved Outdated

Is it cyrillic letter С?

Is it cyrillic letter `С`?

yes, it is, @def should have fixed this.

yes, it is, @def should have fixed this.
assert fp.call_count(["nix-store", "--gc"]) == 1
def test_graphql_nix_collect_garbage_not_authorized_client(client, fp):
assert huey.immediate is True
fp.register(
["nix-env", "-p", "/nix/var/nix/profiles/system", "--delete-generations old"],
stdout="",
)
fp.register(["nix-store", "--gc", "--print-dead"], stdout=OUTPUT_PRINT_DEAD)
fp.register(["nix-store", "--gc"], stdout=OUTPUT_COLLECT_GARBAGE)
response = client.post(
"/graphql",
json={
"query": RUN_NIX_COLLECT_GARBAGE_MUTATION,
},
)
assert_empty(response)
assert (
fp.call_count(
[
"nix-env",
"-p",
"/nix/var/nix/profiles/system",
"--delete-generations old",
]
)
def marked this conversation as resolved Outdated

status update?

status update?
== 0
)
assert fp.call_count(["nix-store", "--gc", "--print-dead"]) == 0
assert fp.call_count(["nix-store", "--gc"]) == 0