Skip to content

Conversation

@VladaZakharova
Copy link
Contributor


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers area:system-tests provider:google Google (including GCP) related issues labels Jul 11, 2024
@potiuk
Copy link
Member

potiuk commented Jul 11, 2024

Some static checks. As usuall - I recommend to have pre-commit installed, it fixes all those problems automatically :)

Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

LGTM overall - static checks should be fixed as Jarek mentioned.
Also, I assume that there was a reason for changing the regions in the system tests - is there any policy for that? (I think that it should be mentioned somwhere)

@VladaZakharova VladaZakharova force-pushed the dataproc-sys-tests branch 2 times, most recently from 6ad0951 to bf35f93 Compare July 23, 2024 11:56
@VladaZakharova
Copy link
Contributor Author

@potiuk @shahar1 hi! thank you for checking this one. Changes in regions and locations were made only to fix the problem in our Composer CI: with previous setup there was a big load on one specific region that was used in all of them. This PR fixes this, but the rest in the test remains the same.
I will try to fix static checks, but there is one that I don't know how to :)
In dataproc_batch we have task that runs cancel_operation() method to cancel any long-running operation. In this case it should be run in parallel with the task that performs that long-running operation (create_batch_4) and since the operation was cancelled, the task itself is failed. Watcher sees the failed task and marks the whole DAG as failed. I was trying to exclude this create_task from the list for watcher, but static check is failing because of that. Do you have some ideas maybe how I can fix it?

@potiuk
Copy link
Member

potiuk commented Jul 23, 2024

I will try to fix static checks, but there is one that I don't know how to :)

In this pre-commit:

index = content.find(WATCHER_APPEND_INSTRUCTION)

Just have a `>> watcher()`` check instead of the full append instructions

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

Labels

area:providers area:system-tests provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants