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

Dataflow sensors - job metrics #12039

Merged
merged 1 commit into from
Nov 17, 2020

Conversation

TobKed
Copy link
Contributor

@TobKed TobKed commented Nov 2, 2020


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:docs provider:google Google (including GCP) related issues labels Nov 2, 2020
@TobKed TobKed marked this pull request as draft November 2, 2020 12:31
@TobKed TobKed changed the title [WIP] Dataflow sensors job metrics. Depends on [11726](https://github.com/apache/airflow/pull/11726) [WIP] Dataflow sensors job metrics. Depends on PR #11726 Nov 2, 2020
@github-actions
Copy link

github-actions bot commented Nov 2, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch 2 times, most recently from e8fe190 to e16a535 Compare November 2, 2020 13:10
@kaxil
Copy link
Member

kaxil commented Nov 4, 2020

Can you please rebased your PR on latest Master since we have applied Black and PyUpgrade on Master

@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch from e16a535 to 7d266ca Compare November 4, 2020 17:21
@github-actions
Copy link

github-actions bot commented Nov 4, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch 2 times, most recently from 915158c to 9f96512 Compare November 5, 2020 08:36
@TobKed TobKed changed the title [WIP] Dataflow sensors job metrics. Depends on PR #11726 Dataflow sensors - job metrics. Depends on PR #11726 Nov 5, 2020
@TobKed TobKed marked this pull request as ready for review November 5, 2020 09:21
@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch from 9f96512 to 03a704c Compare November 5, 2020 09:24
self,
*,
job_id: str,
callback: Callable,
Copy link
Member

Choose a reason for hiding this comment

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

Have you check how well it works with serialisation? Serialising a function can be problematic.cc @kaxil @mik-laj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@turbaszek could you provide some more information how to test it for serialization, please? I am not so familiar with this topic

Copy link
Member

Choose a reason for hiding this comment

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

it is safe for serialization.

@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch 3 times, most recently from 8de885f to 694d67c Compare November 10, 2020 15:04
@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch from 694d67c to 9ceb0e6 Compare November 16, 2020 14:05
@TobKed
Copy link
Contributor Author

TobKed commented Nov 16, 2020

PTAL @aaltay @mik-laj @turbaszek

@TobKed TobKed changed the title Dataflow sensors - job metrics. Depends on PR #11726 Dataflow sensors - job metrics Nov 16, 2020
@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch from 4d04c54 to 263516a Compare November 16, 2020 14:40
@TobKed TobKed force-pushed the dataflow-sensors-job-metrics branch from 263516a to cb16bcb Compare November 16, 2020 15:48
Copy link
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

Nice!

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Nov 17, 2020
@github-actions
Copy link

The PR should be OK to be merged with just subset of tests as it does not modify Core of Airflow. The committers might merge it or can add a label 'full tests needed' and re-run it to run all tests if they see it is needed!

@mik-laj mik-laj merged commit 80a957f into apache:master Nov 17, 2020
@mik-laj mik-laj deleted the dataflow-sensors-job-metrics branch November 17, 2020 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
okay to merge It's ok to merge this PR as it does not require more tests provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants