Skip to content

Conversation

vsbogd
Copy link
Collaborator

@vsbogd vsbogd commented Aug 25, 2025

Allow passing Python environment version. Fix das-cli setup to use this version and virtual environment.
@arturgontijo I have some questions about it:

  1. job fails with Python 3.8 (https://github.com/vsbogd/hyperon-experimental/actions/runs/17214743919) what is the minimal version of Python supported by DAS cli?
  2. DAS cli requires manual requirements installation (python3 -m pip install -r requirements.txt) (see https://github.com/vsbogd/hyperon-experimental/actions/runs/17214573397/job/48834855776) but one would expect python3 -m pip install . should be enough.
  3. Job still fails after all fixes (https://github.com/vsbogd/hyperon-experimental/actions/runs/17214801060/job/48835629376) not sure why
  4. I think the approach with the separate job is not efficient, it requires rebuilding hyperon-experimental before running DAS tests, thus I think it make sense to make this test a separate reusable action and include it into a common.yml with "run-das-tests" flag.

@vsbogd vsbogd requested a review from arturgontijo August 25, 2025 16:46
@arturgontijo
Copy link
Contributor

1 - das-cli minimal version is 3.10.x (ref)
I think we can hardcode python version here as das-cli is just a handy helper and not a part of metta-bus-client

2 - That's odd, will ping my team to improve that.

3 - Let's try pip install it using -e flag (python3 -m pip install -e .), that should be enough

4 - Agree, but I'm not a Github Actions expert so not sure how to do that =/

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

1 - das-cli minimal version is 3.10.x (ref)
I think we can hardcode python version here as das-cli is just a handy helper and not a part of metta-bus-client

Agree

3 - Let's try pip install it using -e flag (python3 -m pip install -e .), that should be enough

Well, I removed -e because in original file after pip install -e the source code directory was being removed and this breaks the das-cli installation.

@arturgontijo
Copy link
Contributor

Well, I removed -e because in original file after pip install -e the source code directory was being removed and this breaks the das-cli installation.

Oh I see, yeah let's not remove the TEMP dir...that should work

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

1 - das-cli minimal version is 3.10.x (ref)

Btw, it probably should be the part of setup.py as well then.

Oh I see, yeah let's not remove the TEMP dir...that should work

If common.config is a part of application should it also be installed?

@arturgontijo
Copy link
Contributor

If common.config is a part of application should it also be installed?

The common module should also be installed/exposed by pip install -e . as it is part of the das-cli source code (https://github.com/singnet/das-toolbox/tree/master/das-cli/src/common/config)

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

I mean should be installed by pip install without -e?

@arturgontijo
Copy link
Contributor

arturgontijo commented Aug 25, 2025

I just ran these lines on Ubuntu 24.04 and it worked:

python3 -m venv /tmp/dastoolbox
source /tmp/dastoolbox/bin/activate

git clone https://github.com/singnet/das-toolbox.git
cd das-toolbox

git checkout tags/0.5.0

cd das-cli/src
python3 -m pip install -r requirements.txt
python3 -m pip install -e .

das-cli --help

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

I just ran these lines on Ubuntu 24.04 and it worked:

Yeah, I see the point. We can add it to the job to see if it works inside GitHub container. But my point is that may be DAS cli installation should be fixed if it is distributed as a standalone application.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

What if someone gets the das-cli code and perform pip install, the installation will be affected by the same issue.

@arturgontijo
Copy link
Contributor

Agree, I'll open an issue in it repo and show this pain points to my teammates tomorrow

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 25, 2025

I will try to fix the job tomorrow.

@vsbogd vsbogd force-pushed the test-nightly branch 7 times, most recently from 2fd09fe to 527084b Compare August 26, 2025 06:25
@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 26, 2025

@arturgontijo do you have an idea why Redis doesn't start on macos images https://github.com/vsbogd/hyperon-experimental/actions/runs/17229876711/job/48881457666 or may be how to get the reason from das-cli?

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 26, 2025

I afraid the reason is das-cli tries to run container but fails because CI job is already inside container.

@arturgontijo
Copy link
Contributor

arturgontijo commented Aug 26, 2025

I afraid the reason is das-cli tries to run container but fails because CI job is already inside container.

Yeah, that's right...I see a possible way to expose Docker would be adding this step before - name: das-toolbox - DAS configuration

      - name: Set up Docker Buildx
        uses: docker/setup-buildx-action@v3

Sorry for all these confusions, we definitely need to improve our tools and docs

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 27, 2025

Looks like Docker is not supported by macos images (except macos-13 x86) and has poor support even on real hardware: https://github.com/marketplace/actions/setup-docker-on-macos#arm64-processors-m-series-used-on-macos-14-images-and-beyond-are-unsupported
Did you try the instruction from PR on MacOSX?

@arturgontijo
Copy link
Contributor

Yesterday I spent a lot trying to do this in singnet/das-toolbox#214 but still only able to properly install das-cli for linux runners =/

I have a very little xp with Actions so maybe I'm missing things...I'll talk to Levi (our devops) today to try to fix that.

Other than that I think the best way would be having the C++ build binaries ready in a release for the most common platforms, I'll also discuss that with the team.

Did you try the instruction from PR on MacOSX?

Yep, I use a macos 15 here and Docker works fine, so I can use das-cli that way too.

My suggestion would be running this only on linux runners (+ windows-latest if you know what am I missing in my das-toolbox's PR) while we wait for the binaries (or other better solution)

@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 27, 2025

Yeah, it looks like support for Docker in macos and linux Github images is not complete. Windows images has Docker preinstalled but doesn't have buildX and setup-buildx-action doesn't help (https://github.com/vsbogd/hyperon-experimental/actions/runs/17262671613). Macos images are arm64 based and they doesn't support Docker (my understanding).

Other than that I think the best way would be having the C++ build binaries ready in a release for the most common platforms, I'll also discuss that with the team.

Well to me it is an open question. At the moment I think that testing DAS integration on all platforms available is not a task for hyperon-experimental repo. It is a DAS team task. And if Python das-cli works on real hardware then there is nothing to bother about. Adding a smoke test for linux platform in hyperon-experimental should be enough.

At the moment it is not possible to run Docker Buildx (which is required
by das-cli) on macos- and windows- GitHub actions images. On the other
hand testing DAS integration comprehensively is not a task of
hyperon-experimental repo. It is suggested to keep only job to make a
smoketest on linux. Be able run this job manually and as a part of
nightly run.
@vsbogd
Copy link
Collaborator Author

vsbogd commented Aug 27, 2025

@arturgontijo please review changes suggested by this PR.

Copy link
Contributor

@arturgontijo arturgontijo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vsbogd vsbogd merged commit e6f9ad6 into trueagi-io:artur-das-test Aug 27, 2025
@vsbogd vsbogd deleted the test-nightly branch September 9, 2025 06:33
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