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

feat: pis stage for the platform orchestration dag #14

Merged
merged 21 commits into from
Sep 9, 2024
Merged

feat: pis stage for the platform orchestration dag #14

merged 21 commits into from
Sep 9, 2024

Conversation

javfg
Copy link
Member

@javfg javfg commented Aug 30, 2024

This PR adds the first stage for the Platform DAG: The one that runs PIS.

In the end it is using Compute Engine machines that are spawned with COOS. They are very fast to spin up (~10 seconds), but quite limited on what they can do.

I've implemented an Airflow Sensor/Operator that is in charge of:

  • Spawning a machine
  • Attaching disks to it
  • Copying config files from a bucket
  • Running a docker image with some arguments
  • Waiting for the exit code of that docker container
  • Repoting a success (exit code 0), or a failure for the task (>0)

Most of the logic is in the gce.py file, which contains that operator.

I also:

  • Added airflow-triggerer service to the docker-compose, as we need it for sensors.
  • Added an ignore_init setting to interrogate so we don't have to add docstrings to __init__ methods (it's annoying and rarely useful).

I'm opening the PR already although work is well underway for the ETL stage, as this way we can split the code into more manageable chunks for review. Shorter reviews find more things to fix!

I'll leave it as pending for now, until the merge of #13, so I can rebase onto it and fix any problems that arise.

@javfg javfg changed the title PIS stage for the Platform orchestration DAG feat: pis stage for the platform orchestration dag Sep 3, 2024
@javfg
Copy link
Member Author

javfg commented Sep 3, 2024

The rationale behind remove interrogate and pydoclint is that we are already checking docstrings with Ruff by using the pydocstyle rules.

Interrogate and Pydoclint are extremely strict, I copypasted a docstring straight from google code and they complain about it. Also, --ignore-init-function and --style google are mutually exclusive and it is a pain to document all __init__ functions.

Let me know if you think this is too big a change.

@javfg
Copy link
Member Author

javfg commented Sep 3, 2024

I cannot reproduce the check error on the poetry lock pre-commit check. We should discuss on loosening those, right now it seems a bit overkill. :)

@javfg javfg marked this pull request as ready for review September 3, 2024 18:19
Copy link
Collaborator

@project-defiant project-defiant left a comment

Choose a reason for hiding this comment

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

Some minor comments, otherwise well done!

Co-authored-by: Szymon Szyszkowski <ss60@mib117351s.internal.sanger.ac.uk>
@project-defiant project-defiant self-requested a review September 9, 2024 14:53
@project-defiant
Copy link
Collaborator

@javfg approved. Nice work!

@project-defiant
Copy link
Collaborator

@javfg can you drop the poetry.lock check. I had the same issue with it on my previous PR.

@javfg javfg merged commit 85109be into dev Sep 9, 2024
2 checks passed
@javfg javfg deleted the pis-stage branch September 9, 2024 15:16
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.

None yet

2 participants