Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for builds on mac+podman #22

Merged
merged 11 commits into from
Aug 21, 2024
Merged

Conversation

etirelli
Copy link

This PR adds changes to support building the project on MacOS + podman.

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@etirelli etirelli changed the title [WIP - do not merge] Support for builds on mac+podman Support for builds on mac+podman Aug 19, 2024
@shalberd
Copy link

shalberd commented Aug 19, 2024

with regards to bootstrapper.py and logging, check this out upstream:

elyra-ai#3227

env variable either in runtime image or in global or per pipeline node env values:

elyra-ai#3227 (comment)

steering whether .log files are written to s3 or not.

with regards to podman builds on arm / Mac Mx and python:

opendatahub-io-contrib/workbench-images#56 (comment)

@etirelli regarding what you did in this PR for logging script output correctly and not necessarily putting the .log output to S3: I added a new kfp bootstrapper.py upstream

and documentation

https://github.com/shalberd/elyra/blob/5262b5bfc094885446134e9326a7844b93e6a4bd/docs/source/user_guide/env-variables-file-based-nodes.md

specifically for that issue with the .log file output to S3 vs logging script output to STDOUT only.

https://github.com/elyra-ai/elyra/blob/main/docs/source/user_guide/env-variables-file-based-nodes.md

https://github.com/elyra-ai/elyra/blob/main/elyra/kfp/bootstrapper.py#L51

https://github.com/elyra-ai/elyra/blob/main/elyra/kfp/bootstrapper.py#L502

@etirelli
Copy link
Author

@shalberd thank you for the suggestion. However, without the changes from this PR, even when using --arch option, the build fails on my mac. Since these changes only affect the image used for local development (inner loop), I would like to eventually merge them (or an equivalent fix) into the codebase.

@caponetto
Copy link

I was looking into the CI error on Elyra Tests / Test Server (3.9) (pull_request):

elyra/pipeline/kfp/kfp_authentication.py:30: in <module>
    from kfp.client import KF_PIPELINES_SA_TOKEN_ENV
E   ModuleNotFoundError: No module named 'kfp.client'

The import has been changed on #1 from kfp.auth to kfp.client only on this fork, and this project is supposed to use kfp>=2.0.0 (see here).

Consider that the latest kfp-tekton has the following requirement:

kfp-tekton==1.9.3
├── kfp [required: >=1.8.10,<1.8.23]

Although kfp-tekton has been removed in #1, it can still be found as a test dependency here. And the CI installs this dependency with make test-dependencies.

So if you run pip freeze | grep kfp on this fork after installing the test dependencies, you'd get:

kfp==1.8.22
...
kfp-tekton==1.9.3
...

This is the same result that you see on the Version Snapshot step on the CI run.

Note that, since kfp-tekton is installed, it forces kfp to be 1.8.22, not >=2.0.0.
As a result, the const KF_PIPELINES_SA_TOKEN_ENV is still exported from kfp.auth, not kfp.client.

@harshad16 harshad16 changed the base branch from dspv2 to jupyterlab4 August 21, 2024 12:44
@harshad16 harshad16 merged commit 65eeb9a into opendatahub-io:jupyterlab4 Aug 21, 2024
9 of 13 checks passed
@shalberd
Copy link

shalberd commented Aug 21, 2024

@harshad16 is what @caponetto writes regarding CI tests relevant for a follow-up PR?

And, of course interesting for me, will the changes related to Jupyterlab v4 support and KFP v2 make it into upstream elyra?

paulovmr pushed a commit that referenced this pull request Sep 26, 2024
* applying changes to build the dev image for mac

* applying changes to build the dev image for mac

* Updating Dockerfile to compile elyra on mac

* implementing changes for the build process to work on a mac + podman

* implementing changes for the build process to work on a mac + podman

* Fixing dockerfile to work on mac

* implementing changes for the build process to work on a mac + podman

* Fixing dockerfile to work on mac

* fixing linter

* Upgrading to Node 20

* Undoing yarn.lock change

---------

Co-authored-by: Eder Ignatowicz <ignatowicz@gmail.com>
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.

5 participants