aleph-im / aleph-vm

Aleph.im VM execution engine
MIT License
43 stars 18 forks source link

Fix 199 ruff errors #689

Closed hoh closed 2 months ago

hoh commented 2 months ago

Explain what problem this PR is resolving

The python linter ruff is not enable on this project because it raises too many errors to fix at once. At the time of writing, ruff check src raises 375 errors.

Self proofreading checklist

Not relevant for these changes.

Changes

The first commit contains the result of ruff check src --fix, which contains safe automatic fixes to the codebase.

I then ran ruff check src --fix --unsafe-fixes and added the following commits with the changes that I reviewed manually and deem ready to commit:

How to test

Run ruff check src or hatch run testing:ruff.

ruff check src on the code found 176 errors after the changes.

hoh commented 2 months ago

I had the same reflection: Not sure why the CI failed on this PR while it passed for another one around the same time... :thinking:

We did not add ruff to the CI because the remaining 176 errors contain non trivial changes and I don't know of any tool similar to codecov but that would look at the increment/decrement in linter errors in the codebase. Seemed like all or nothing.

olethanh commented 2 months ago

it seem ruff and black don't agree on some formatting.

Syncing dependencies
cmd [1] | black --check --diff .
--- /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/conf.py    2024-09-03 16:53:16.680400+00:00
+++ /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/conf.py    2024-09-03 16:55:05.973700+00:00
@@ -284,13 +284,11 @@
would reformat /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/conf.py
     BENCHMARK_FAKE_DATA_PROGRAM = Path(abspath(join(__file__, "../../../../examples/example_fastapi")))

     FAKE_DATA_MESSAGE = Path(abspath(join(__file__, "../../../../examples/program_message_from_aleph.json")))
     FAKE_DATA_DATA: Path | None = Path(abspath(join(__file__, "../../../../examples/data/")))
     FAKE_DATA_RUNTIME = Path(abspath(join(__file__, "../../../../runtimes/aleph-debian-12-python/rootfs.squashfs")))
-    FAKE_DATA_VOLUME: Path | None = Path(
-        abspath(join(__file__, "../../../../examples/volumes/volume-venv.squashfs"))
-    )
+    FAKE_DATA_VOLUME: Path | None = Path(abspath(join(__file__, "../../../../examples/volumes/volume-venv.squashfs")))

     # Tests on instances

     TEST_INSTANCE_ID: str | None = Field(
         default=None,  # TODO: Use a valid item_hash here
--- /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/models.py  2024-09-03 16:53:16.680400+00:00
+++ /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/models.py  2024-09-03 16:55:06.231799+00:00
@@ -184,11 +184,13 @@
would reformat /home/runner/work/aleph-vm/aleph-vm/src/aleph/vm/models.py
             if self.resources:
                 # Already prepared
                 return

             self.times.preparing_at = datetime.now(tz=timezone.utc)
-            resources: AlephProgramResources | AlephInstanceResources | AlephQemuResources | AlephQemuConfidentialInstance
+            resources: (
+                AlephProgramResources | AlephInstanceResources | AlephQemuResources | AlephQemuConfidentialInstance
+            )
             if self.is_program:
                 resources = AlephProgramResources(self.message, namespace=self.vm_hash)
             elif self.is_instance:
                 if self.hypervisor == HypervisorType.firecracker:
                     resources = AlephInstanceResources(self.message, namespace=self.vm_hash)

I have also checked running hatch fmt which also run ruff but I think under another config and it make other changes. Such as

-async def get_message(hash_: str) -> Optional[dict]:
+async def get_message(hash_: str) -> dict | None:

edit: just noticed they are in the test directory

olethanh commented 2 months ago

I have pushed more ruff fix and a black run

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 63.76147% with 79 lines in your changes missing coverage. Please review.

Project coverage is 62.32%. Comparing base (fbbbf3d) to head (24c6594). Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/aleph/vm/orchestrator/payment.py 0.00% 16 Missing :warning:
src/aleph/vm/models.py 68.00% 8 Missing :warning:
src/aleph/vm/utils/__init__.py 38.46% 8 Missing :warning:
src/aleph/vm/controllers/qemu/instance.py 53.84% 6 Missing :warning:
src/aleph/vm/network/interfaces.py 40.00% 6 Missing :warning:
src/aleph/vm/orchestrator/chain.py 44.44% 5 Missing :warning:
src/aleph/vm/orchestrator/views/authentication.py 44.44% 5 Missing :warning:
...c/aleph/vm/hypervisors/qemu_confidential/qemuvm.py 0.00% 4 Missing :warning:
src/aleph/vm/orchestrator/cli.py 0.00% 4 Missing :warning:
src/aleph/vm/guest_api/__main__.py 25.00% 3 Missing :warning:
... and 8 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #689 +/- ## ========================================== - Coverage 62.69% 62.32% -0.38% ========================================== Files 69 69 Lines 6069 6073 +4 Branches 642 638 -4 ========================================== - Hits 3805 3785 -20 - Misses 2113 2139 +26 + Partials 151 149 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.