Skip to content

change: uv ify the project#20

Merged
adulau merged 1 commit into
vulnerability-lookup:mainfrom
qjerome:uv-ify-project
Feb 5, 2026
Merged

change: uv ify the project#20
adulau merged 1 commit into
vulnerability-lookup:mainfrom
qjerome:uv-ify-project

Conversation

@qjerome

@qjerome qjerome commented Jan 28, 2026

Copy link
Copy Markdown
Contributor
  • make the project installable with uv
  • change dockerfile to run with uv
  • simplified docker-compose.yml installation
  • overall project simplifications

- make the project installable with uv
- change dockerfile to run with uv
- simplified docker-compose.yml installation
- overall project simplifications
@adulau adulau merged commit d251af9 into vulnerability-lookup:main Feb 5, 2026
2 checks passed
@adulau

adulau commented Feb 5, 2026

Copy link
Copy Markdown
Member

Thank you!

@oh2fih

oh2fih commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Post-merge note: this PR simplifies the project with a uv-based workflow, but it breaks existing usage patterns—running scripts directly, Docker Compose with mounted configs, and SystemD deployments no longer work.

“Make the project installable with uv” sounds like an addition, but it effectively replaces previous workflows. Ideally, uv would have been optional, allowing, for example, running lookup.py and server.py directly.

As a result, the current README (especially Installation and Command line) and documentation site no longer reflect accurate installation or runtime instructions. At a minimum, the docs should be updated, and a migration guide would be a helpful extra.

@adulau

adulau commented Feb 11, 2026

Copy link
Copy Markdown
Member

Indeed good point.

@qjerome could you adjust in a new PR to ensure the previous workflow still works for the other users?

@oh2fih oh2fih left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Much of this PR reads like a broad refactor that removes or replaces existing functionality without preserving backward compatibility. A cleaner approach would be to revert it, migrate to uv for packaging only, and then introduce small, carefully reasoned, backward-compatible changes to the rest of the project — with clear commit messages explaining each change and corresponding updates to the user-facing documentation to keep it in sync.

@@ -1,31 +1,24 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This prevents running the importer directly.

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.

fix there: f649298

@@ -1,16 +1,12 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This prevents running the lookup directly.

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.

fix there: f649298

@@ -59,14 +56,17 @@ def on_post(self, req, resp):
return

cpeGuesser = CPEGuesser()
try:
r = cpeGuesser.guessCpe(q["query"])[:1][0][1]
except:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While broadly catching exceptions was not ideal, this change removes exception handling entirely.

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.

There are already lots of changes in the pipe for that part of the code so I will not include direct fixes.

@rg-atte rg-atte Feb 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It seems that the intent was to remove the [:1][0][1] from the guessCpe result like in the lookup.py change but now it seems like it does both the length checks and cpeGuesser.guessCpe(q["query"])[:1][0][1] (now without try except)which to me seems like it would break the intended functionality entirely even if it didn't cause an exception. at least based on the diff and what the change in lookup.py did.

Comment thread Dockerfile
COPY etc /etc
COPY lib lib
COPY docker/entrypoint.sh entrypoint.sh
uv run cpe-import

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This runs the import only once at startup.

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.

Wasn't already the case in the previous implementation ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Discussion moved to #20 (comment)

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.

Maybe I am missing something here but AFAIK docker only runs the entrypoint, there is no systemd scheduling tasks.


[Timer]
Unit=cpeguesser.service
OnCalendar=*-*-* 0/12:00:00

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As you can see from here, this used to run periodically, but that behavior has been removed without explanation.

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.

I didn't really see where it was used in the previous implementation, this was my main motivation to remove it. Maybe you could explain me how it was used so that I can eventually put it back into place if necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you referring to the public instance at https://cpe-guesser.cve-search.org/search?

The SystemD unit directory was introduced in #7. While that PR primarily added the units without extensive explanation, they appear to have been intended as example configurations for running the project outside of Docker under SystemD. They provide guidance for users who prefer managing services directly on a host system. Since not all deployments rely on Docker, those files supported alternative runtime setups.

That implementation is far from perfect and could be improved:

  • As with this PR, it would have helped if the purpose of these units had been clearly explained in the documentation.

  • The periodic updates were one issue this setup addressed, and I would like similar functionality to remain available across all supported installation variants. I would not bind updates to cron jobs or OnCalendar specifications; instead, I prefer an approach that triggers updates when a certain interval has passed since the last update, which is less disruptive for the upstream than enforcing a fixed time across installations.

@@ -59,14 +56,17 @@ def on_post(self, req, resp):
return

cpeGuesser = CPEGuesser()
try:
r = cpeGuesser.guessCpe(q["query"])[:1][0][1]
except:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While broadly catching exceptions was not ideal, this change removes exception handling entirely, which may introduce new failure modes.

Comment thread .python-version
@@ -0,0 +1 @@
3.13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

uv should be able to manage Python versions without requiring the repository to pin an exact version.

@qjerome qjerome Feb 11, 2026

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.

Version pinning is admitted as a good practice as it is supposed to guarantee at least a working setup for the tools shipped by the project. If the version is not wanted, one is free to ignore it. Moreover Python version pinning is not used when using the package as dependency. Rather the package manager relies on the requires-python field in the pyproject.toml file. I can remove it, yet I don't really understand the motivation behind this removal.

@oh2fih oh2fih Feb 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file appears to follow a documented approach, as outlined in UV’s Python version files guide, so it doesn’t seem inherently problematic. My perspective may differ, as I tend to prioritize portability across environments rather than a fixed, containerized setup. This is more of a library-development mindset, whereas the cybersecurity community often focuses on a narrower, deployment-specific usage of these tools.

Comment thread pyproject.toml
[project]
name = "cpe-guesser"
version = "0.1.0"
description = "Add your description here"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This appears to be a placeholder. Was it copied from a template or generated automatically?

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.

fix description here: 6e605a9

@qjerome qjerome mentioned this pull request Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants