Skip to content

Conversation

@Yikun
Copy link
Member

@Yikun Yikun commented Aug 3, 2022

What changes were proposed in this pull request?

Pin roxygen2 to 7.2.0 to make sparkr job work

Why are the changes needed?

R job failed due to checking for missing documentation entries

Does this PR introduce any user-facing change?

No

How was this patch tested?

All CI passed (lint/sparkr/pyspark)

@github-actions github-actions bot added the BUILD label Aug 3, 2022
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for taking a look at this, @Yikun .

@Yikun
Copy link
Member Author

Yikun commented Aug 3, 2022

  • The root reason of sparkr job failed should be roxygen2 7.2.0 --> 7.2.1
  • Docker cache not working (looks like is outdate for some reason) since 2 days before
  • Then do a real time build in each job, roxygen2 is upgraded in real time build job.

Solution: merge this if all CI passed (This will also do a cache refresh to make cache back again)

@Yikun Yikun mentioned this pull request Aug 3, 2022
@dongjoon-hyun dongjoon-hyun marked this pull request as ready for review August 3, 2022 21:05
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you, @Yikun .
Since master branch has been broken for a while, I converted this to a normal PR and merged to master to recover it.

@HyukjinKwon
Copy link
Member

Awesome!!!!!

@Yikun
Copy link
Member Author

Yikun commented Aug 4, 2022

@dongjoon-hyun @HyukjinKwon Thanks, I do a local test in my repo all works well!

I also take a deep look on why cache is outdated: the ubuntu:20.04 hash changes before and after cache outdate. (looks like FROM is a specific case in docker registry cache, it's not based on static text match? or some other reason to trigger the full update? Need to do a confirm. it will find the real hash of image rather than static text match)

There might some improvement in followup:

@LuciferYang
Copy link
Contributor

Thanks @Yikun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants