Skip to content

AWS Resources - #63

Open
robeertosv wants to merge 3 commits into
junquera:masterfrom
robeertosv:master
Open

AWS Resources#63
robeertosv wants to merge 3 commits into
junquera:masterfrom
robeertosv:master

Conversation

@robeertosv

Copy link
Copy Markdown
Contributor

ARN, S3 Buckets, Access Keys de IAM, EC2 Instances y RDS endpoints.

@robeertosv

Copy link
Copy Markdown
Contributor Author

Add support for ARN, S3 Buckets, Access Keys de IAM, EC2 Instances y RDS endpoints detection. Also, migrates the library management from poetry to uv

@CarlosLannister CarlosLannister left a comment

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.

Thanks for this PR, @robeertosv. The new AWS and BTC/altcoin detectors are solid, and the uv migration is well documented. Before merging, a few issues need fixing:


🔴 Critical: import base58 → NameError at runtime

In restalker/restalker.py, the import was changed:

-import based58
+import base58

But the new _is_valid_base58_check() still calls based58.b58decode_check() — the name based58 no longer exists in scope. This will crash.

Additionally, based58 (Rust, PyPI) and base58 (pure-Python, PyPI) are different packages. The project depends on based58 for its b58decode_check and b58encode_check functions (used in tests too). The base58 package does not expose these.

Fix: Revert to import based58 in restalker.py:5, and restore based58 as the dependency in pyproject.toml, requirements.txt, requirements-gpu-cuda.txt, and requirements-gpu-rocm.txt.


🔴 Critical: CPU-only PyTorch index removed

requirements.txt lost the --index-url https://download.pytorch.org/whl/cpu directive. The [tool.uv] section has a comment about CPU defaults but no actual config to enforce it. Without this, uv pip install -r requirements.txt pulls the GPU torch (~2.2 GB) instead of CPU-only (~500 MB).

Fix: Restore --index-url https://download.pytorch.org/whl/cpu before the torch>=2.0.0 line in requirements.txt, or add index-url under [tool.uv] in pyproject.toml.


🟡 Missing: AWS classes not added to restalker/__init__.py

The 7 new BTC/altcoin classes were properly added to the imports and __all__, but the 5 new AWS classes — AWS_ARN, S3_Bucket, AWS_Access_Key, EC2_Instance, RDS_Endpoint — are missing. Users would need to import from restalker.restalker directly, which is inconsistent with the rest of the library.

Fix: Add AWS classes to both the import section and __all__ in restalker/__init__.py.


Once these are addressed this will be good to go. Thanks for the contribution!

CarlosLannister

This comment was marked as duplicate.

CarlosLannister

This comment was marked as duplicate.

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.

2 participants