Skip to content

Conversation

@kszucs
Copy link
Member

@kszucs kszucs commented Feb 3, 2020

Executing the verification script can take quite some time, so creating a new environment in case if anything fails is time consuming.
Let the script reuse the same build directory for source release verification.

Need to export TMPDIR environment variable. @kou shall we use an argument instead?

@kszucs kszucs changed the title ARROW-7750 [Release] Make the source release verification script restartable ARROW-7750: [Release] Make the source release verification script restartable Feb 3, 2020
@github-actions
Copy link

github-actions bot commented Feb 3, 2020

@nealrichardson
Copy link
Member

I attempted to use this when verifying the release candidate but did not realize I had to set an env var to have a persistent directory. It otherwise succeeded, but then the rm -rf $TMPDIR at the end failed to delete the dir in /tmp.

+ TEST_SUCCESS=yes
+ echo 'Release candidate looks good!'
Release candidate looks good!
+ exit 0
+ cleanup
+ '[' yes = yes ']'
+ rm -fr /var/folders/jf/t7_zsd293791z5qwt2_bv8640000gn/T/
rm: /var/folders/jf/t7_zsd293791z5qwt2_bv8640000gn/T/: Operation not permitted

Not critical since the verification already succeeded I guess.

nealrichardson added a commit to nealrichardson/arrow that referenced this pull request Feb 3, 2020
Copy link
Member Author

@kszucs kszucs left a comment

Choose a reason for hiding this comment

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

+1

@kszucs kszucs closed this in a605417 Feb 4, 2020
kszucs added a commit that referenced this pull request Feb 4, 2020
This addition follows the pattern of `test_source_distribution` (as it is in #6344). There are also two error message patches to make them consistent with everywhere else that references this env var (though FWIW `testing/data` is still not correct in the verification script, it's `arrow-testing/data` 🤷‍♂).

Closes #6345 from nealrichardson/flight-testing-data and squashes the following commits:

a29e88a <Krisztián Szűcs> factor out testing repository cloning
df9ef25 <Neal Richardson> Move addition and fix lint
e165d54 <Neal Richardson> Make sure macOS wheel verification has test data

Lead-authored-by: Neal Richardson <[email protected]>
Co-authored-by: Krisztián Szűcs <[email protected]>
Signed-off-by: Krisztián Szűcs <[email protected]>
@kou
Copy link
Member

kou commented Feb 4, 2020

Environment variable is OK but the name, TMPDIR, isn't suitable.
Because TMPDIR is too generic. Many commands such as mktemp refer TMPSDIR. And macOS defines TMPDIR by default.

ARROW_TMPDIR, TEST_TMPDIR or something is better.

@kszucs
Copy link
Member Author

kszucs commented Feb 4, 2020

kszucs added a commit that referenced this pull request Feb 7, 2020
…tartable

Executing the verification script can take quite some time, so creating a new environment in case if anything fails is time consuming.
Let the script reuse the same build directory for source release verification.

Need to export `TMPDIR` environment variable. @kou shall we use an argument instead?

Closes #6344 from kszucs/restartable-verification and squashes the following commits:

6d4723d <Krisztián Szűcs> Support for restarting the release verification script

Authored-by: Krisztián Szűcs <[email protected]>
Signed-off-by: Krisztián Szűcs <[email protected]>
kszucs added a commit that referenced this pull request Feb 7, 2020
This addition follows the pattern of `test_source_distribution` (as it is in #6344). There are also two error message patches to make them consistent with everywhere else that references this env var (though FWIW `testing/data` is still not correct in the verification script, it's `arrow-testing/data` 🤷‍♂).

Closes #6345 from nealrichardson/flight-testing-data and squashes the following commits:

a29e88a <Krisztián Szűcs> factor out testing repository cloning
df9ef25 <Neal Richardson> Move addition and fix lint
e165d54 <Neal Richardson> Make sure macOS wheel verification has test data

Lead-authored-by: Neal Richardson <[email protected]>
Co-authored-by: Krisztián Szűcs <[email protected]>
Signed-off-by: Krisztián Szűcs <[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.

3 participants