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

Create scripts to launch benchmark testing to Google cloud #496

Merged
merged 29 commits into from
Jun 30, 2022

Conversation

dimberman
Copy link
Collaborator

@dimberman dimberman commented Jun 28, 2022

Description

What is the current behavior?

Currently users are running benchmark tests on their local machines. This is a) inconsistent as much of the time might be tied to network time and b) bad as it discourages consistently running benchmarks (E.g. as a daily job or as a part of CI).

closes: #432
this PR is following up on the PR: #471

What is the new behavior?

Now users will have the ability to launch a GKE cluster, push an image for the astro SDK, and launch the job to run on GKE

Does this introduce a breaking change?

No

Checklist

  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary
  • Be able to run the benchmark, with different configurations, in the cloud (GCP and GKE)

@dimberman dimberman changed the base branch from issue-432 to main June 29, 2022 01:43
@kaxil
Copy link
Collaborator

kaxil commented Jun 29, 2022

Closing-reopening to see if it triggeres CI

@kaxil kaxil closed this Jun 29, 2022
@kaxil kaxil reopened this Jun 29, 2022
@dimberman dimberman closed this Jun 29, 2022
@dimberman dimberman reopened this Jun 29, 2022
@kaxil kaxil closed this Jun 29, 2022
@kaxil kaxil reopened this Jun 29, 2022
@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #496 (a1e89bc) into main (375998e) will not change coverage.
The diff coverage is n/a.

❗ Current head a1e89bc differs from pull request most recent head 06a7a23. Consider uploading reports for the commit 06a7a23 to get more accurate results

@@           Coverage Diff           @@
##             main     #496   +/-   ##
=======================================
  Coverage   92.02%   92.02%           
=======================================
  Files          40       40           
  Lines        1342     1342           
  Branches      182      182           
=======================================
  Hits         1235     1235           
  Misses         85       85           
  Partials       22       22           

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 375998e...06a7a23. Read the comment docs.

pyproject.toml Outdated Show resolved Hide resolved
tests/benchmark/README.md Outdated Show resolved Hide resolved
tests/benchmark/config.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

Did the script run fine on GKE now?

Just have some minor questions that I added as comments

@dimberman
Copy link
Collaborator Author

@kaxil I can confirm that this runs smoothly on GKE. I was able to do the entire flow from beginning to end. Glad to demo tomorrow if you like.

@dimberman
Copy link
Collaborator Author

@kaxil I'm gonna run this script one more time and merge if everything works.

Comment on lines 40 to 45
"conn_id": "bigquery",
"file_type": "ndjson",
"name": "one_gb",
"path": "gs://astro-sdk/benchmark/trimmed/stackoverflow/stackoverflow_posts_1g.ndjson",
"rows": 385817,
"size": "1G"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove this from here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kaxil I'm not sure hwo that happened. I added it back in.

@kaxil kaxil merged commit 23e2215 into main Jun 30, 2022
@kaxil kaxil deleted the dimberman-issue-432 branch June 30, 2022 20:00
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.

Change benchmark tool to run in the cloud (GCP)
3 participants