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.

Sign up for GitHub

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

Jump to bottom

✨ D3B-674 Add cavatica related gf fields #681

Merged
merged 1 commit into from
Sep 5, 2024
Merged

✨ D3B-674 Add cavatica related gf fields #681

merged 1 commit into from
Sep 5, 2024

Conversation

chris-s-friedman
Copy link
Contributor

No description provided.

@chris-s-friedman chris-s-friedman requested a review from a team as a code owner August 28, 2024 19:43
znatty22
znatty22 reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

Need to add a test to the genomic file API tests. If you add new fields to

kf-api-dataservice/tests/genomic_file/test_genomic_file_resources.py

Line 512 in f96f93a

def _new_genomic_file(client):
, that should take care of test_genomic_file_resources.py

znatty22
znatty22 reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

Also please add the new fields to data.json. This gets used in the general API tests which test for things like filters, bad values, etc:

kf-api-dataservice/tests/data.json

Line 112 in f96f93a

"/genomic-files": {

znatty22
znatty22 reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@znatty22 znatty22 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We also need the db schema migration:

# this creates the migration
flask db migrate -m "name of migration"

# this applies the migration to the db
flask db upgrade

Make sure to add the semantic version number in the docstring of the migration py file

@jenkins-kf jenkins-kf force-pushed the D3B-674 branch 3 times, most recently from d316438 to 4429c05 Compare September 4, 2024 15:17
znatty22
znatty22 approved these changes Sep 5, 2024
View reviewed changes
Copy link
Member

@znatty22 znatty22 left a comment

Choose a reason for hiding this comment

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

Looks good, just one minor comment on the migration docstring!

migrations/versions/869f891d97ad_add_cavatica_gf_fields.py Outdated Show resolved Hide resolved
@chris-s-friedman @znatty22
✨ Add cavatica related gf fields
7e5336c 
🗃️ add tests and flask migration

🎨 fix formating to old version

🐛 forgot a comma

Update migrations/versions/869f891d97ad_add_cavatica_gf_fields.py

Co-authored-by: Natasha Singh <dukedesi22@gmail.com>
@chris-s-friedman chris-s-friedman merged commit 182f4b2 into master Sep 5, 2024
4 checks passed
@chris-s-friedman chris-s-friedman deleted the D3B-674 branch September 5, 2024 16:46
chris-s-friedman added a commit that referenced this pull request Sep 9, 2024
@chris-s-friedman
🏷️ Release 1.33.0
22e80d9 
## Release 1.33.0

### Summary

- Emojis: ✨ x3, ⬆️ x1
- Categories: Additions x3, Ops x1

### New features and changes

- [#679](#679) - ✨ AD-2303 add amount and amount_units - [1ac116a](1ac116a) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#680](#680) - ✨ Add has_matched_normal and collection_id - [f3da17b](f3da17b) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#681](#681) - ✨ D3B-674 Add cavatica related gf fields - [182f4b2](182f4b2) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#682](#682) - ⬆️ Bump Werkzeug from 0.15.3 to 0.15.5 - [645d888](645d888) by [chris-s-friedman](https://github.com/chris-s-friedman)
@chris-s-friedman chris-s-friedman mentioned this pull request Sep 9, 2024
Closed
chris-s-friedman added a commit that referenced this pull request Sep 9, 2024
@chris-s-friedman
🏷️ Release 1.33.0
dd1e02f 
## Release 1.33.0

### Summary

- Emojis: ? x2, ✨ x3, 👨🏻‍💻 x1, ⬆️ x1
- Categories: Additions x3, Ops x1, Other Changes x3

### New features and changes

- [#686](#686) -  🐛 Update GH Action to use correct branch - [4646d95](4646d95) by [haynescd](https://github.com/haynescd)
- [#679](#679) - ✨ AD-2303 add amount and amount_units - [1ac116a](1ac116a) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#680](#680) - ✨ Add has_matched_normal and collection_id - [f3da17b](f3da17b) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#685](#685) -  👷 Add Github Action to build and deploy dev image when merged to master - [56cd60c](56cd60c) by [haynescd](https://github.com/haynescd)
- [#684](#684) - 👨🏻‍💻 Always bind all interfaces when app runs in docker-compose - [7370bbd](7370bbd) by [znatty22](https://github.com/znatty22)
- [#681](#681) - ✨ D3B-674 Add cavatica related gf fields - [182f4b2](182f4b2) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#682](#682) - ⬆️ Bump Werkzeug from 0.15.3 to 0.15.5 - [645d888](645d888) by [chris-s-friedman](https://github.com/chris-s-friedman)
chris-s-friedman added a commit that referenced this pull request Sep 9, 2024
@chris-s-friedman
🏷️ Release 1.33.0
61e558d 
## Release 1.33.0

### Summary

- Emojis: ? x3, ✨ x3, 👨🏻‍💻 x1, ⬆️ x1
- Categories: Additions x3, Ops x1, Other Changes x4

### New features and changes

- [#688](#688) -  🐛 Update Tag of dataservice image - [ae8831e](ae8831e) by [haynescd](https://github.com/haynescd)
- [#686](#686) -  🐛 Update GH Action to use correct branch - [4646d95](4646d95) by [haynescd](https://github.com/haynescd)
- [#679](#679) - ✨ AD-2303 add amount and amount_units - [1ac116a](1ac116a) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#680](#680) - ✨ Add has_matched_normal and collection_id - [f3da17b](f3da17b) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#685](#685) -  👷 Add Github Action to build and deploy dev image when merged to master - [56cd60c](56cd60c) by [haynescd](https://github.com/haynescd)
- [#684](#684) - 👨🏻‍💻 Always bind all interfaces when app runs in docker-compose - [7370bbd](7370bbd) by [znatty22](https://github.com/znatty22)
- [#681](#681) - ✨ D3B-674 Add cavatica related gf fields - [182f4b2](182f4b2) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#682](#682) - ⬆️ Bump Werkzeug from 0.15.3 to 0.15.5 - [645d888](645d888) by [chris-s-friedman](https://github.com/chris-s-friedman)
@chris-s-friedman chris-s-friedman mentioned this pull request Sep 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HuangXiaoyan0106 HuangXiaoyan0106 HuangXiaoyan0106 approved these changes

@znatty22 znatty22 znatty22 approved these changes

@yuankunzhu yuankunzhu Awaiting requested review from yuankunzhu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-s-friedman @znatty22 @HuangXiaoyan0106