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

Make GitPod works #2688

Merged
merged 18 commits into from
Jun 28, 2023
Merged

Make GitPod works #2688

merged 18 commits into from
Jun 28, 2023

Conversation

noklam
Copy link
Contributor

@noklam noklam commented Jun 14, 2023

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

The whole Gitpod settings is not working at all

  • installation is two steps, which makes the installed library very different from what you will get in the CI
  • make build-docs cannot be run - error
  • pre-commit is broken, it stuck with some corrupted virtualenv and you need to reinstall it manually.

Related to ##2586, but not a complete fix.

Development notes

  • make lint work nows again
  • make build-docs works

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

@noklam noklam changed the title Update .gitpod.yml Make GitPod works Jun 14, 2023
@noklam
Copy link
Contributor Author

noklam commented Jun 22, 2023

@astrojuanlu I removed some of the setup and make it simpler. The current setup simply doesn't work any more as we are not running in the pre-build mode.

Do you have any idea to make the current setup to a Docker setup? This PR would at least make sure pre-commit works and the libraries installed properly. make build-docs is still not working unfortunately with VideoDataSet having some more complicated cv dependency, I haven't got time to fix this yet.

@noklam
Copy link
Contributor Author

noklam commented Jun 22, 2023

ImportError: libGL.so.1: cannot open shared object file: No such file or directory
>>> from kedro_datasets.video import *
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspace/.pyenv_mirror/user/current/lib/python3.8/site-packages/kedro_datasets/video/__init__.py", line 5, in <module>
    from .video_dataset import VideoDataSet
  File "/workspace/.pyenv_mirror/user/current/lib/python3.8/site-packages/kedro_datasets/video/video_dataset.py", line 12, in <module>
    import cv2
  File "/workspace/.pyenv_mirror/user/current/lib/python3.8/site-packages/cv2/__init__.py", line 8, in <module>
    from .cv2 import *
ImportError: libGL.so.1: cannot open shared object file: No such file or directory

@astrojuanlu
Copy link
Member

👀 Let me have a look

@astrojuanlu
Copy link
Member

  • Right after the initial task finishes, kedro_datasets is not installed.
  • make build-docs installs kedro-datasets[all]~=1.4.0 (pulled from the docs optional dependencies group) and immediately fails on VideoDataSet
  • Then sudo apt-get update && sudo apt-get install -y --no-install-recommends libgl1 as advised in https://stackoverflow.com/a/74501248/554319 fixes the import, and make build-docs gets back to almost working, but fails when generating the output because mmdc (the Mermaid CLI) is not installed
    • So, this setup needs either NPM preinstalled, or make use of conda environments. Maybe worth going back to the full image?

@noklam
Copy link
Contributor Author

noklam commented Jun 26, 2023

  • Right after the initial task finishes, kedro_datasets is not installed.

Do we really need kedro_datasets installed for kedro or is it required by building docs?

  • make build-docs installs kedro-datasets[all]~=1.4.0 (pulled from the docs optional dependencies group) and immediately fails on VideoDataSet
  • Then sudo apt-get update && sudo apt-get install -y --no-install-recommends libgl1 as advised in https://stackoverflow.com/a/74501248/554319 fixes the import, and make build-docs gets back to almost working, but fails when generating the output because mmdc (the Mermaid CLI) is not installed

sudo will require the Dockerfile approach? I try running sudo in terminal it doesn't work.

  • So, this setup needs either NPM preinstalled, or make use of conda environments. Maybe worth going back to the full image?

I think that's fine.

@astrojuanlu
Copy link
Member

  • Right after the initial task finishes, kedro_datasets is not installed.

Do we really need kedro_datasets installed for kedro or is it required by building docs?

They're needed to build the docs.

  • make build-docs installs kedro-datasets[all]~=1.4.0 (pulled from the docs optional dependencies group) and immediately fails on VideoDataSet
  • Then sudo apt-get update && sudo apt-get install -y --no-install-recommends libgl1 as advised in https://stackoverflow.com/a/74501248/554319 fixes the import, and make build-docs gets back to almost working, but fails when generating the output because mmdc (the Mermaid CLI) is not installed

sudo will require the Dockerfile approach? I try running sudo in terminal it doesn't work.

I just tested it and worked fine for me in the terminal 🤔 Could you share a screenshot?

@noklam
Copy link
Contributor Author

noklam commented Jun 26, 2023

It does work in the terminal. I don't know what've I done last week 😅 https://medium.com/@pidugusundeep5/installing-packages-with-apt-get-on-gitpod-a70f0c6b1cf4 This also convince me that sudo is only possible with Docker

@noklam
Copy link
Contributor Author

noklam commented Jun 26, 2023

They're needed to build the docs.
We just need the kedro_datasets package but not all the dependencies right? The current setup should create a new project from pandas-iris, we should install the project dependencies and kedro_datasets will be installed, I will fix it.

@astrojuanlu
Copy link
Member

Building a complete version of the docs without warnings requires kedro-datasets[all] indeed. It has always been this way actually, but now it's official:

kedro/setup.py

Line 107 in fd8162d

"kedro-datasets[all]~=1.4.0",

That doesn't mean that the Gitpod image should install all of them (after all, make build-docs would take care of it).

@noklam
Copy link
Contributor Author

noklam commented Jun 26, 2023

This is the closest I can get now, it failed midway with some mermaid error. With the full image the startup time is also longer without pre-built.

Warning, treated as error:
mermaid code 'flowchart TD\n A[Start] --> B{Do you prefer developing your projects in notebooks?}\n B -->|Yes| C[Use a Databricks workspace to develop a Kedro project]\n B -->|No| D{Are you a beginner with Kedro?}\n D -->|Yes| E[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n D -->|No| F{Do you have advanced project requirements
e.g. CI/CD, scheduling, production-ready, complex pipelines, etc.?}\n F -->|Yes| G{Is rapid development needed for your project needs?}\n F -->|No| H[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n G -->|Yes| I[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n G -->|No| J[Use a Databricks job to deploy a Kedro project]': Mermaid exited with error:
[stderr]
b'\x1b[1m\x1b[43m\x1b[30m\n Puppeteer old Headless deprecation warning:\x1b[0m\x1b[33m\n In the near feature headless: true will default to the new Headless mode\n for Chrome instead of the old Headless implementation. For more\n information, please see https://developer.chrome.com/articles/new-headless/.\n Consider opting in early by passing headless: "new" to puppeteer.launch()\n If you encounter any bugs, please report them to https://github.com/puppeteer/puppeteer/issues/new/choose.\x1b[0m\n\n\nError: Failed to launch the browser process!\n/home/gitpod/.cache/puppeteer/chrome/linux-1108766/chrome-linux/chrome: error while loading shared libraries: libatk-1.0.so.0: cannot open shared object file: No such file or directory\n\n\nTROUBLESHOOTING: https://pptr.dev/troubleshooting\n\n at Interface.onClose (file:///home/gitpod/.nvm/versions/node/v18.16.0/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/@puppeteer/browsers/lib/esm/launch.js:253:24)\n at Interface.emit (node:events:525:35)\n at Interface.close (node:internal/readline/interface:533:10)\n at Socket.onend (node:internal/readline/interface:259:10)\n at Socket.emit (node:events:525:35)\n at endReadableNT (node:internal/streams/readable:1359:12)\n at process.processTicksAndRejections (node:internal/process/task_queues:82:21)\n\n'
[stdout]
b''

@noklam noklam closed this in #2731 Jun 27, 2023
@astrojuanlu
Copy link
Member

astrojuanlu commented Jun 27, 2023

Guessing you didn't want to close this? 😄

@noklam noklam reopened this Jun 27, 2023
@noklam
Copy link
Contributor Author

noklam commented Jun 27, 2023

@astrojuanlu Ah, accidentally used the keywords fix and GitHub think it is smart...

@noklam noklam marked this pull request as ready for review June 27, 2023 11:18
@noklam
Copy link
Contributor Author

noklam commented Jun 27, 2023

@astrojuanlu It is working now when I start a new Gitpod and do make build-docs.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

LGTM with a nitpick!

.gitpod.yml Outdated Show resolved Hide resolved
Signed-off-by: Nok <[email protected]>
@noklam noklam enabled auto-merge (squash) June 27, 2023 13:29
@noklam noklam disabled auto-merge June 27, 2023 13:38
@noklam noklam enabled auto-merge (squash) June 27, 2023 13:38
Copy link
Contributor

@ankatiyar ankatiyar left a comment

Choose a reason for hiding this comment

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

I tried make build-docs and make lint and it's working!

@noklam noklam merged commit 6e5e4e1 into main Jun 28, 2023
2 of 3 checks passed
@noklam noklam deleted the noklam-patch-2 branch June 28, 2023 10:00
debugger24 pushed a commit to debugger24/kedro that referenced this pull request Jun 28, 2023
* update base image

Signed-off-by: Nok <[email protected]>

* update gitpod image and apt-get

Signed-off-by: Nok <[email protected]>

* update docker

Signed-off-by: Nok <[email protected]>
---------

Signed-off-by: Nok <[email protected]>
Signed-off-by: debugger24 <[email protected]>
astrojuanlu added a commit that referenced this pull request Jun 28, 2023
* FIX: Typo in cli help

Signed-off-by: debugger24 <[email protected]>

* Make GitPod works (#2688)

* update base image

Signed-off-by: Nok <[email protected]>

* update gitpod image and apt-get

Signed-off-by: Nok <[email protected]>

* update docker

Signed-off-by: Nok <[email protected]>
---------

Signed-off-by: Nok <[email protected]>
Signed-off-by: debugger24 <[email protected]>

* Remove `DeprecatedClassMeta` in favor of `getattr` (#2724)

* Complete build requirements

Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>

* Add timeout for e2e process waiting

Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>

* Remove DeprecatedClassMeta in favor of getattr

Fix kedro-org/kedro-starters#137

Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>

* Ignore deprecation warnings in our own code

Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>

---------

Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>
Co-authored-by: Juan Luis Cano Rodríguez <[email protected]>
Co-authored-by: Deepyaman Datta <[email protected]>
Signed-off-by: debugger24 <[email protected]>

---------

Signed-off-by: debugger24 <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Juan Luis Cano Rodríguez <[email protected]>
Co-authored-by: Nok Lam Chan 陳諾林 <[email protected]>
Co-authored-by: Deepyaman Datta <[email protected]>
Co-authored-by: Juan Luis Cano Rodríguez <[email protected]>
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