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(config): 24.06 data release fixes #639

Merged
merged 5 commits into from
Jun 13, 2024

Conversation

project-defiant
Copy link
Contributor

@project-defiant project-defiant commented Jun 11, 2024

✨ Context

24.06 data release

🛠 What does this PR implement

Addresses issues found during the 24.06 data release:

  • typos in airflow configuration
  • grch37_to_grch_38_chain_file parameter not passed to the LDIndexStep from LDIndexStepConfig causing step failure.

🙈 Missing

🚦 Before submitting

  • Do these changes cover one single feature (one change at a time)?
  • Did you read the contributor guideline?
  • Did you make sure to update the documentation with your changes?
  • Did you make sure there is no commented out code in this PR?
  • Did you follow conventional commits standards in PR title and commit messages?
  • Did you make sure the branch is up-to-date with the dev branch?
  • Did you write any new necessary tests?
  • Did you make sure the changes pass local tests (make test)?
  • Did you make sure the changes pass pre-commit rules (e.g poetry run pre-commit run --all-files)?

@project-defiant project-defiant changed the title feat(config): typos fix feat(config): 24.06 data release fixes Jun 11, 2024
Copy link
Contributor

@ireneisdoomed ireneisdoomed left a comment

Choose a reason for hiding this comment

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

There is a bug in the LD index path.
We really need to migrate this configuration to the Airflow layer sooner than later.

@github-actions github-actions bot added size-S and removed size-XS labels Jun 11, 2024
@project-defiant
Copy link
Contributor Author

@ireneisdoomed I have moved the ld_index data in following fashion:
old static asset from gs://genetics_etl_python_playground/static_assets/ld_index/ -> gs://genetics_etl_python_playground/static_assets/ld_index/2.1.1-exclude-est-pop/ (as this is also 2.1.1 but with missing EST population)
new ld_index generated in the 24.06 data release from gs://genetics_etl_python_playground/output/python_etl/parquet/XX.XX/ld_index/2.1.1/ -> gs://genetics_etl_python_playground/static_assets/ld_index/2.1.1

Also I have updated the path in the config to match the new ld_index.

Copy link
Contributor

@ireneisdoomed ireneisdoomed left a comment

Choose a reason for hiding this comment

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

Thanks!

@project-defiant project-defiant merged commit 3c8ce58 into dev Jun 13, 2024
4 checks passed
project-defiant added a commit that referenced this pull request Jun 14, 2024
* feat(config): typos fix
* fix(config): moved ld_index dataset to static assets

---------

Co-authored-by: Szymon Szyszkowski <ss60@mib117351s.internal.sanger.ac.uk>
project-defiant pushed a commit that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants