Add -e flag to some GH workflow yml files #16959

ydshieh · 2022-04-27T08:33:25Z

What does this PR do?

Current 2 GitHub actions workflow yml use pip install [.dev]. This installs transformers in /home/runner/venv/lib/python3.6/site-packages, and this is cached. In future job runs, the cache is restored, and that transformers version is used - instead of the latest commit, i.e. we want to use /home/runner/work/transformers/transformers/src.

Without this PR, I have trouble after updating add_new_model.py (to change test model folders from tests/ to tests/models/) because the add_new_model.py from /home/runner/venv/lib/python3.6/site-packages would be used, which would put the test template models under tests/

This PR makes sure the latest transformers is used by using pip install -e [.dev], and builds a new cache with it.

Add -e flag to some GH workflow yml files
change cache key in order to make the change effective
add a check on transformers location

HuggingFaceDocBuilderDev · 2022-04-27T10:27:11Z

The documentation is not available anymore as the PR was closed or merged.

ydshieh · 2022-04-27T10:54:08Z

.github/workflows/add-model-like.yml

        with:
          path: ~/venv/
-          key: v2-tests_model_like-${{ hashFiles('setup.py') }}
+          key: v3-tests_model_like-${{ hashFiles('setup.py') }}


in order to make the change effective (then saved to a new cache)

ydshieh · 2022-04-27T10:55:59Z

.github/workflows/add-model-like.yml

          python -m venv ~/venv && . ~/venv/bin/activate
          pip install --upgrade pip!=21.3
-          pip install .[dev]
+          pip install -e .[dev]


this is the main change in order to use /home/runner/work/transformers/transformers/src

This doesn't usually work, as an editable install writes some content in some folders, and this is not allowed on GitHub action pull-request (for security reasons, since the user have put code on their own in their PR).

I have no idea why the check passes now, maybe it's not executing this version of the test script.

ydshieh · 2022-04-27T10:56:39Z

.github/workflows/add-model-like.yml

+      - name: Check transformers location
+        run: |
+          . ~/venv/bin/activate
+          transformer_loc=$(pip show transformers | grep "Location: " | cut -c11-)
+          transformer_repo_loc=$(pwd .)
+          if [ "$transformer_loc" != "$transformer_repo_loc/src" ]; then
+              echo "transformers is from $transformer_loc but it shoud be from $transformer_repo_loc/src."
+              echo "A fix is required. Stop testing."
+              exit 1
+          fi


add a check of the package location, so we won't be fooled in the future.

gante

LGTM, thanks for the fix 👍

ydshieh · 2022-04-27T11:14:29Z

The added check will produce something like (if the transformers is not from the expected location)

sgugger

As discussed offline, good for me, but let's be super vigilant after merging and ready to revert.

Thanks for working on this!

ydshieh · 2022-04-27T16:05:21Z

after the new cache is built the first time and job completes (pass the check I added too), the next run when we have

Cache restored from key: v3-tests_model_like-ce386d6c28d7afcca58dc875de2ef1b7477e8246a0bfdb6ff4de0eb222eafef2

the check failed with

transformers is from  but it shoud be from /home/runner/work/transformers/transformers/src.
A fix is required. Stop testing.

which means pip show transformers gives empty location!

I will try to make it work - I really like to have this test.
~~(But things should work now if we just remove this test)~~

ydshieh · 2022-04-27T16:25:26Z

Confirmed this (with -e) currently not working for the subsequentially run (i.e. cache loaded).

I still think there might be some workaround, let me try. Set to draft for now

currently error

Traceback (most recent call last):
  File "/home/runner/venv/bin/transformers-cli", line 33, in <module>
    sys.exit(load_entry_point('transformers', 'console_scripts', 'transformers-cli')())
  File "/home/runner/venv/bin/transformers-cli", line [22](https://github.com/huggingface/transformers/runs/6197315688?check_suite_focus=true#step:7:22), in importlib_load_entry_point
    for entry_point in distribution(dist_name).entry_points
  File "/home/runner/venv/lib/python3.6/site-packages/importlib_metadata/__init__.py", line 815, in distribution
    return Distribution.from_name(distribution_name)
  File "/home/runner/venv/lib/python3.6/site-packages/importlib_metadata/__init__.py", line 430, in from_name
    raise PackageNotFoundError(name)
importlib_metadata.PackageNotFoundError: No package metadata was found for transformers
Error: Process completed with exit code 1.

ydshieh · 2022-04-27T16:58:55Z

.github/workflows/add-model-like.yml

+        # make `transformers` available as package (required since we use `-e` flag) and check it indeeds from the repo.
        run: |
          . ~/venv/bin/activate
+          python setup.py build install


@sgugger let me know if you have any comment regarding this new change. 🙏

(Lysandre and gante are OK with it on a Slack message)

Sounds good!

LysandreJik

LGTM

LysandreJik · 2022-04-27T18:28:55Z

.github/workflows/add-model-like.yml

+        # make `transformers` available as package (required since we use `-e` flag) and check it's indeed from the repo.
+        run: |
+          . ~/venv/bin/activate
+          python setup.py build install


Usually when installing it with -e, you should do python setup.py develop, no?
If this works well here, fine by me!

Change it to develop as it makes more sense. Thank you for the information!

* Add -e flag * add check * create new keys * run python setup.py build install * add comments * change to develop Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>

ydshieh added 7 commits April 27, 2022 10:31

Add -e flag

82b068f

test - to be revert

b74d65b

temp change to generate new cache - to be reverted

4c83be2

temp change to generate new cache - to be reverted

dc02968

add check

21284a7

fix

f064507

clean-up

40e2b18

create new keys

0f9562b

ydshieh requested review from LysandreJik, gante and sgugger April 27, 2022 10:48

ydshieh commented Apr 27, 2022

View reviewed changes

gante approved these changes Apr 27, 2022

View reviewed changes

sgugger approved these changes Apr 27, 2022

View reviewed changes

ydshieh marked this pull request as draft April 27, 2022 16:25

ydshieh added 2 commits April 27, 2022 18:48

run python setup.py build install

cc98a41

add comments

f84a5fd

ydshieh commented Apr 27, 2022

View reviewed changes

ydshieh marked this pull request as ready for review April 27, 2022 17:03

fix typo

4620ba3

LysandreJik approved these changes Apr 27, 2022

View reviewed changes

change to develop

7aa2bad

ydshieh merged commit 992996e into huggingface:main Apr 27, 2022

ydshieh deleted the fix_git_workflow_yml branch April 27, 2022 19:44

Add -e flag to some GH workflow yml files #16959

Add -e flag to some GH workflow yml files #16959

Uh oh!

Conversation

ydshieh commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Uh oh!

HuggingFaceDocBuilderDev commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ydshieh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

sgugger Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

gante left a comment

Choose a reason for hiding this comment

Uh oh!

ydshieh commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sgugger left a comment

Choose a reason for hiding this comment

Uh oh!

ydshieh commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ydshieh commented Apr 27, 2022

currently error

Uh oh!

ydshieh Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgugger Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

LysandreJik left a comment

Choose a reason for hiding this comment

Uh oh!

LysandreJik Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ydshieh commented Apr 27, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Apr 27, 2022 •

edited

Loading

ydshieh commented Apr 27, 2022 •

edited

Loading

ydshieh commented Apr 27, 2022 •

edited

Loading

ydshieh Apr 27, 2022 •

edited

Loading