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.

Sign up for GitHub

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

Jump to bottom

chore(ci): fix flakiness, misc improvements #7605

Merged
merged 9 commits into from
Mar 17, 2023
Merged

chore(ci): fix flakiness, misc improvements #7605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f154aa5
chore(ci): add info log to find flakiness
anshbansal Mar 16, 2023
f10b8cc
try to run through with python instead of python3
anshbansal Mar 16, 2023
38d7536
remove cache to fix python issue
anshbansal Mar 16, 2023
cb62994
add python change
anshbansal Mar 16, 2023
3154349
remove venv caching
anshbansal Mar 16, 2023
1114241
separate package install to find test time
anshbansal Mar 16, 2023
ba6139c
add check for upper bound of python
anshbansal Mar 16, 2023
1b538af
fix problem with upper bound check
anshbansal Mar 16, 2023
2198d56
Merge branch 'master' into aseem-add-info-to-find-flakiness
jjoyce0510 Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 12 additions & 11 deletions .github/workflows/check-datahub-jars.yml
View file
Open in desktop
Expand Up @@ -24,6 +24,15 @@ concurrency:

jobs:
check_jars:
strategy:
fail-fast: false
matrix:
command:
[
"datahub-client",
"datahub-protobuf",
"spark-lineage"
]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Expand All @@ -37,15 +46,7 @@ jobs:
- uses: actions/setup-python@v4
with:
python-version: "3.7"
- name: check datahub-client jar
- name: check ${{ matrix.command }} jar
run: |
./gradlew :metadata-integration:java:datahub-client:build
./gradlew :metadata-integration:java:datahub-client:javadoc
- name: check spark-lineage jar
run: |
./gradlew :metadata-integration:java:spark-lineage:build
./gradlew :metadata-integration:java:spark-lineage:javadoc
- name: check datahub-protobuf jar
run: |
./gradlew :metadata-integration:java:datahub-protobuf:build
./gradlew :metadata-integration:java:datahub-protobuf:javadoc
./gradlew :metadata-integration:java:${{ matrix.command }}:build --info
./gradlew :metadata-integration:java:${{ matrix.command }}:javadoc
14 changes: 0 additions & 14 deletions .github/workflows/docker-unified.yml
View file
Open in desktop
Expand Up @@ -490,20 +490,6 @@ jobs:
if: ${{ needs.setup.outputs.publish != 'true' }}
with:
image: ${{ env.DATAHUB_UPGRADE_IMAGE }}:${{ needs.setup.outputs.unique_tag }}
- name: Calculate pip install plan
id: "packages_checksum"
run: |
pip install pip -U # only 22.2 and above contains the --dry-run flag

# only the last line of the output is the packages that will be installed
pip install --dry-run -r ./smoke-test/requirements.txt | tail -n 1 > /tmp/would_be_installed.txt
CHECKSUM=$(shasum /tmp/would_be_installed.txt | awk '{print $1}')
echo "packages_checksum=$CHECKSUM" >> $GITHUB_OUTPUT
- uses: actions/cache@v3
id: cache-venv
with:
path: ./smoke-test/venv/
key: smoke-venv-${{ runner.os }}-${{ steps.packages_checksum.outputs.packages_checksum }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we comment this out instead? Do you actually know what the issue was here or just disabling because of issues?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no difference in run time. See master and this branch's runs. No time difference is there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Weird - that probably means the caching wasn't even working?

- name: run quickstart
env:
DATAHUB_TELEMETRY_ENABLED: false
Expand Down
22 changes: 3 additions & 19 deletions .github/workflows/metadata-ingestion.yml
View file
Open in desktop
Expand Up @@ -54,26 +54,10 @@ jobs:
cache: 'pip'
- name: Install dependencies
run: ./metadata-ingestion/scripts/install_deps.sh
- name: Calculate pip install plan
id: "packages_checksum"
run: |
cd metadata-ingestion
pip install pip -U # only 22.2 and above contains the --dry-run flag

# only the last line of the output is the packages that will be installed
pip install --dry-run -e .[dev] ${{ matrix.extraPythonRequirement }} | tail -n 1 > /tmp/would_be_installed.txt
CHECKSUM=$(shasum /tmp/would_be_installed.txt | awk '{print $1}')
echo "packages_checksum=$CHECKSUM" >> $GITHUB_OUTPUT
- name: print dependencies
id: print-dependency
run: cat /tmp/would_be_installed.txt
- uses: actions/cache@v3
id: cache-venv
with:
path: ./metadata-ingestion/venv/
key: ${{ runner.os }}-venv-${{ steps.packages_checksum.outputs.packages_checksum }}-${{ matrix.python-version }}
- name: Install package
run: ./gradlew :metadata-ingestion:installPackageOnly
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this just for debugging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. Wanted to see how much time installing the package takes separately to the tests.

- name: Run metadata-ingestion tests (extras ${{ matrix.extraPythonRequirement }})
run: ./gradlew -Pextra_pip_requirements='${{ matrix.extraPythonRequirement }}' :metadata-ingestion:${{ matrix.command }}
run: ./gradlew -Pextra_pip_requirements='${{ matrix.extraPythonRequirement }}' :metadata-ingestion:${{ matrix.command }} -x :metadata-ingestion:installPackageOnly
- name: pip freeze show list installed
if: always()
run: source metadata-ingestion/venv/bin/activate && pip freeze
Expand Down
1 change: 1 addition & 0 deletions metadata-ingestion/.gitignore
View file
Open in desktop
Expand Up @@ -140,3 +140,4 @@ dmypy.json

# Generated classes
src/datahub/metadata/
.preflight_sentinel
33 changes: 25 additions & 8 deletions metadata-ingestion/build.gradle
View file
Open in desktop
Expand Up @@ -12,7 +12,8 @@ if (!project.hasProperty("extra_pip_requirements")) {
}

task checkPythonVersion(type: Exec) {
commandLine python_executable, '-c', 'import sys; assert sys.version_info >= (3, 7)'
commandLine python_executable, '-c',
'import sys; assert (3, 11) > sys.version_info >= (3, 7), f"Python version {sys.version_info[:2]} not allowed"'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change message to "Is too old"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean? If someone is using 3.11 it won't be too old but too new.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh i missed the 3.11 part - then this is fine

}

task environmentSetup(type: Exec, dependsOn: checkPythonVersion) {
Expand All @@ -22,10 +23,22 @@ task environmentSetup(type: Exec, dependsOn: checkPythonVersion) {
}

task runPreFlightScript(type: Exec, dependsOn: environmentSetup) {
def sentinel_file = ".preflight_sentinel"
outputs.file(sentinel_file)
commandLine "scripts/datahub_preflight.sh"
commandLine 'bash', '-c', "touch ${sentinel_file}"
}

task installPackage(type: Exec, dependsOn: runPreFlightScript) {
task installPackageOnly(type: Exec, dependsOn: runPreFlightScript) {
def sentinel_file = "${venv_name}/.build_install_package_only_sentinel"
inputs.file file('setup.py')
outputs.dir("${venv_name}")
outputs.file(sentinel_file)
commandLine 'bash', '-x', '-c', "${venv_name}/bin/pip install -e ."
commandLine 'bash', '-c', "touch ${sentinel_file}"
}

task installPackage(type: Exec, dependsOn: installPackageOnly) {
inputs.file file('setup.py')
outputs.dir("${venv_name}")
commandLine 'bash', '-x', '-c', "${venv_name}/bin/pip install -e . ${extra_pip_requirements}"
Expand All @@ -40,25 +53,27 @@ task codegen(type: Exec, dependsOn: [environmentSetup, installPackage, ':metadat
task install(dependsOn: [installPackage, codegen])

task installDev(type: Exec, dependsOn: [install]) {
def sentinel_file = "${venv_name}/.build_install_dev_sentinel"
inputs.file file('setup.py')
outputs.dir("${venv_name}")
outputs.file("${venv_name}/.build_install_dev_sentinel")
outputs.file(sentinel_file)
commandLine 'bash', '-c',
"source ${venv_name}/bin/activate && set -x && " +
"${venv_name}/bin/pip install -e .[dev] ${extra_pip_requirements} && " +
"./scripts/install-sqlalchemy-stubs.sh && " +
"touch ${venv_name}/.build_install_dev_sentinel"
"touch ${sentinel_file}"
}

task installAll(type: Exec, dependsOn: [install]) {
def sentinel_file = "${venv_name}/.build_install_all_sentinel"
inputs.file file('setup.py')
outputs.dir("${venv_name}")
outputs.file("${venv_name}/.build_install_all_sentinel")
outputs.file(sentinel_file)
commandLine 'bash', '-c',
"source ${venv_name}/bin/activate && set -x && " +
"${venv_name}/bin/pip install -e .[all] ${extra_pip_requirements} && " +
"./scripts/install-sqlalchemy-stubs.sh && " +
"touch ${venv_name}/.build_install_all_sentinel"
"touch ${sentinel_file}"
}


Expand Down Expand Up @@ -111,11 +126,12 @@ task testQuick(type: Exec, dependsOn: [installDev, ':metadata-models:generateJso
}

task installDevTest(type: Exec, dependsOn: [install]) {
def sentinel_file = "${venv_name}/.build_install_dev_test_sentinel"
inputs.file file('setup.py')
outputs.dir("${venv_name}")
outputs.file("${venv_name}/.build_install_dev_test_sentinel")
outputs.file(sentinel_file)
commandLine 'bash', '-c',
"${venv_name}/bin/pip install -e .[dev,integration-tests] ${extra_pip_requirements} && touch ${venv_name}/.build_install_dev_test_sentinel"
"${venv_name}/bin/pip install -e .[dev,integration-tests] ${extra_pip_requirements} && touch ${sentinel_file}"
}

def testFile = hasProperty('testFile') ? testFile : 'unknown'
Expand Down Expand Up @@ -176,6 +192,7 @@ clean {
delete 'generated'
delete '.mypy_cache'
delete '.pytest_cache'
delete '.preflight_sentinel'
}
clean.dependsOn cleanPythonCache

Expand Down