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

Test on GitHub Actions #420

Merged
merged 7 commits into from Nov 16, 2020
Merged

Test on GitHub Actions #420

merged 7 commits into from Nov 16, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Nov 12, 2020

This tests with the same as Travis CI, and I replaced nightly with 3.10-dev, the nearest equivalent.

Example: https://github.com/hugovk/twitter/actions/runs/360319693

They fail for me because:

details: {u'errors': [{u'message': u'User is over daily status update limit.', u'code': 185}]}

Will try again tomorrow.

@RouxRC
Copy link
Member

RouxRC commented Nov 12, 2020

Yep, too many tests ran today with the account, will need to retry later

@coveralls
Copy link

coveralls commented Nov 13, 2020

Coverage Status

Coverage increased (+0.2%) to 28.494% when pulling 802ef61 on hugovk:add-gha into cc91099 on sixohsix:master.

@RouxRC
Copy link
Member

RouxRC commented Nov 15, 2020

Hey Hugo,
Sorry with my merges and your push, I guess finding a window for the tests was not easy. Can you ask for a rebuild on your github actions tryout?
Then if it's successful, I guess we should prepare a merge by removing the .travis file first and the relative badge in the readme.
Do you know if there is a way to keep some kind of badge as well with GH Actions?

@hugovk
Copy link
Member Author

hugovk commented Nov 15, 2020

Hi, no problem!

I restarted a build:

https://github.com/hugovk/twitter/actions/runs/363545104

Some passed, some failed. I'll check the failures tomorrow.

Yes, it has a badge, will be something like:

![Test](https://github.com/sixohsix/twitter/workflows/Test/badge.svg)

@RouxRC
Copy link
Member

RouxRC commented Nov 15, 2020

Looking at the test account, it seems like all python versions run simultaneously which might be an issue with the API limits, it is possible to set them to run successively instead of in parallel?

@hugovk
Copy link
Member Author

hugovk commented Nov 16, 2020

Good idea! Yes, it's possible with jobs.<job_id>.strategy.max-parallel:

https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstrategymax-parallel

Updated, and that helped:


2.7 and PyPy2 fails due to:

CoverallsException: Not on TravisCI. You have to provide either repo_token in .coveralls.yml or set the COVERALLS_REPO_TOKEN env var.

I'll check that (I'm mostly using Codecov on other projects). Maybe using https://github.com/coverallsapp/github-action or https://github.com/AndreMiras/coveralls-python-action instead will help.

@hugovk hugovk force-pushed the add-gha branch 2 times, most recently from 14f5984 to 6cb365a Compare November 16, 2020 15:53
@hugovk
Copy link
Member Author

hugovk commented Nov 16, 2020

@RouxRC
Copy link
Member

RouxRC commented Nov 16, 2020

Yeah ! \o/
So I guess we're nearly good to merge?
Do you wanna add the badge change and the travis removal within the PR first, and maybe rename the action as "Tests" with a final "s"?

@hugovk
Copy link
Member Author

hugovk commented Nov 16, 2020

Yes, I think we're good to merge! I've remove .travis.yml and updated the badge here.

Copy link
Member

@RouxRC RouxRC left a comment

Choose a reason for hiding this comment

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

There's just one leftover line I'm not sure about

And out of curiosity, could you tell me what do the pip-cache and actions/cache serve for exactly?

Thanks again!

README Outdated Show resolved Hide resolved
@hugovk
Copy link
Member Author

hugovk commented Nov 16, 2020

And out of curiosity, could you tell me what do the pip-cache and actions/cache serve for exactly?

Sure, they're for caching the pip downloads, so they don't need to be downloaded from PyPI each time.

PyPI costs $800k/month in hosting! (In March 2020, will be higher now):

I believe this is generously donated by Fastly, but it would be good to help reduce it where possible, and CIs will contribute quite a lot.


The config is in two parts:

      - name: Get pip cache dir
        id: pip-cache
        run: |
          echo "::set-output name=dir::$(pip cache dir)"

First, we check the pip cache directory by calling pip cache dir and saving the output in a variable called dir; we can refer back to it in later steps as steps.pip-cache.outputs.dir.

Strictly speaking, this is only needed when testing on multiple operating systems, we could hardcode it as we're only testing on Linux.

More info:

      - name: Cache
        uses: actions/cache@v2
        with:
          path: ${{ steps.pip-cache.outputs.dir }}
          key:
            ${{ matrix.os }}-${{ matrix.python-version }}-v1-${{ hashFiles('**/setup.py') }}
          restore-keys: |
            ${{ matrix.os }}-${{ matrix.python-version }}-v1-

Next, we use the https://github.com/actions/cache action to fetch cache at the start, and potentially save a new cache at the end.

path is the pip dir we saved from the first bit.

key is an ID generated and used when saving the cache. We put in the OS and Python version, to make sure we don't mix and match caches between other OS and Python versions (we could remove OS, as we test use Linux right now). We also hash the setup.py file so a new cache is created when dependencies change. Often dependencies are listed in setup.py or requirements.txt. Now I check, there's actually none listed in setup.py and there is no requirements.txt, so this could be removed, although I doubt it makes too much difference. Finally, the v1 is there as an easy way to invalidate an old cache and force a new one to be created; because GHA doesn't have a UI to delete old caches.

restore-keys is a list of substrings to match old cache names, in case there's none matching key.


So we could reduce those two chunks to something like:

      - name: Cache
        uses: actions/cache@v2
        with:
          path: ~/.cache/pip
          key:
            ${{ matrix.python-version }}-v1-${{ hashFiles('**/setup.py') }}
          restore-keys: |
            ${{ matrix.python-version }}-v1-

@RouxRC
Copy link
Member

RouxRC commented Nov 16, 2020

Ha great, so it's a cache across successive action runs, I wouldn't even have thought it was possible and was not understanding how caching would work across different python versions :)
Makes total sense now, thanks a lot for the explanations, very useful!

@RouxRC RouxRC merged commit 727f1f8 into python-twitter-tools:master Nov 16, 2020
@RouxRC
Copy link
Member

RouxRC commented Nov 16, 2020

And of course we already sent too many tweets for the day, will rerun the action for the badge to display properly tomorrow!

@hugovk hugovk deleted the add-gha branch November 16, 2020 19:52
@RouxRC
Copy link
Member

RouxRC commented Nov 17, 2020

And now we're finally good! :)
Thx again for this long run :)

@hugovk
Copy link
Member Author

hugovk commented Nov 17, 2020

Great, happy to help! And thank you for maintaining this library!

@RouxRC
Copy link
Member

RouxRC commented Nov 17, 2020

Now we really need to bother @sixohsix one more time and ask for a release, or better rights on pypi ;)

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.

None yet

3 participants