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

fix: build-indicators.sh looks for setup.py and pyproject.toml #2048

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

dshemetov
Copy link
Contributor

@dshemetov dshemetov commented Sep 11, 2024

Description

Our Jenkins build script previously looked in every indicator folder and if it finds setup.py it installs the indicator. I changed this check to pyproject.toml, in anticipation of #2016, but that is premature. This adds the setup.py check in, but also future proofs by adding a check for pyproject.toml as well.

Changelog

Maintaining previous behavior and future-proofing Jenkins/build-indicators.sh.

Associated Issue(s)

  • Addresses #(issue)

Copy link
Contributor

@minhkhul minhkhul 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. The rest of #2014 is intentional right?

Copy link
Contributor

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

can we get a little more explanation in the PR description, for posterity?

jenkins/build-indicator.sh Outdated Show resolved Hide resolved
@dshemetov
Copy link
Contributor Author

dshemetov commented Sep 11, 2024

@melange396: Added the context to the OP. I did the readability improvement you suggested, but it turns out that we have at least one indicator that doesn't have a setup.py (backfill_corrections) and the more direct phrasing causes it to return with an exit code of 1 and stop the rest of the Jenkins build. So I had to get more verbose.
@minhkhul: This should be it, the rest of #2014 should be fine.

@dshemetov dshemetov changed the title fix: build-indicators.sh looks for setup.py again fix: build-indicators.sh looks for setup.py and pyproject.toml Sep 11, 2024
@minhkhul minhkhul merged commit 6f841bb into main Sep 11, 2024
16 checks passed
@dshemetov dshemetov deleted the dshemetov-patch-1 branch September 11, 2024 21:48
minhkhul added a commit that referenced this pull request Sep 12, 2024
* doc: add logger examples to README and format

* deployment + staging details

* wrap lines

* Update _template_python/INDICATOR_DEV_GUIDE.md

Co-authored-by: David Weber <[email protected]>

* Update _template_python/INDICATOR_DEV_GUIDE.md

Co-authored-by: David Weber <[email protected]>

* Update _template_python/INDICATOR_DEV_GUIDE.md

Co-authored-by: David Weber <[email protected]>

* Update INDICATOR_DEV_GUIDE.md with mysql syntax

* Update INDICATOR_DEV_GUIDE.md reorder instruction on cron jobs

* switch from input_file to input_dir (#1995)

* adjust nssp max_age

* add pyproject.toml

* made changes based on suggestion

* update ci config

* separate new line

Co-authored-by: Dmitry Shemetov <[email protected]>

* fix: add explicit boto and moto dependencies to indicators

* trying to get package data to show

* installing dev package before testing in ci

* adding dependency

* move comment to right package

* Update _delphi_utils_python/pyproject.toml

Co-authored-by: Dmitry Shemetov <[email protected]>

* Revert "Clean up delphi_utils "

* refactoring date parsing to seperate file

* first implimentation

* small cleanup and adding missing data

* changed wording and some logic

* made test more explicit

* add missing file

* cleaning up tests

* consolidating tests into one

* fixed wording

* consolidating test/fixture

* clean up test

* Update google_symptoms/delphi_google_symptoms/patch.py

remove old code; not used anymore

Co-authored-by: nmdefries <[email protected]>

* clean up lint

* remove teardown

* cleaning and renaming

* more explicit branching for regular run vs patching, renaming, cleaning

* cleanup and rewrite logic

* avoid linking keywords in PR template

* Update _delphi_utils_python/README.md

Co-authored-by: george <[email protected]>

* Refactor Jenkins pipeline

- Remove usage of parallel in the build and package stages
- Retain usage of parallel in the deploy stages
- Separate build and package into separate scripts to support build-only stages
- Add a build-only stage for dev/feature branches

* Add separate build and package scripts

* Update script comments

* Remove unused script

* changes based on suggestion

* standardizing output dir in test

* changing back to session scope

* more logging and update params

* adding back conditional for num_export_days

* test_run does not like fixtures

* make deep copies of params fixtures

* linting

* test patch check if num_export_days is set or not

* patch fn takes params as an arg, like run_module

* list values added using actual day as an example

* move pull date creation into pull_gs_data

* don't modify num_export_days; define start/end date in terms of it and lag

* refer to params by name; patch_flag -> custom_run flag

* use test data based on recent actual indicator run

* say where gold test data came from

* not all date_utils tests need metadata

* formatting

* fn docs

* lint and cleanup

* adding more tests

* removed extra lag and made test more robust

* export_start_date is optional since the first available date is baked in

* need to account for historical / current run for adding lag

* adding validation script

* fix typo

* fixing typo in test

* lint

* sample run for validation

* remove unused input

Co-authored-by: nmdefries <[email protected]>

* remove unused import

* added progress chunk to limit logging (#2024)

* added progress chunk to limit logging

* lint

* comment based on suggestion

* add hhs geo level (#2036)

* Add discussion of patching functionality to indicator manual (#2031)

* basic patching info

* custom_run

* add hhs to geo section

* de-emphasize R in contributing doc, and add links

* ref/issue date intro

* naming and force push standards

* secret and params links

* Clean up delphi_utils (take 2) (#2014)

* add pyproject.toml

* made changes based on suggestion

* update ci config

* separate new line

Co-authored-by: Dmitry Shemetov <[email protected]>

* fix: add explicit boto and moto dependencies to indicators

* trying to get package data to show

* installing dev package before testing in ci

* adding dependency

* move comment to right package

* Update _delphi_utils_python/pyproject.toml

Co-authored-by: Dmitry Shemetov <[email protected]>

* more suggestion

* adding pyproject.toml to template

* Update changehc/Makefile

* Update claims_hosp/Makefile

* Update doctor_visits/Makefile

* Update google_symptoms/Makefile

* Update hhs_hosp/Makefile

* Update nchs_mortality/Makefile

* Update nssp/Makefile

* Update nwss_wastewater/Makefile

* Update quidel_covidtest/Makefile

* Update sir_complainsalot/Makefile

* fix: dependence bugs and cleanup
* pandas[excel]<2 doesn't work, go back to xlrd and comment
* add mock to changehc
* add requests
* remove setup.py from template

* build: fix Jenkins build script

---------

Co-authored-by: Amaris Sim <[email protected]>
Co-authored-by: aysim319 <[email protected]>
Co-authored-by: Dmitry Shemetov <[email protected]>

* fix: build-indicators.sh looks for setup.py and pyproject.toml (#2048)

* fix: build-indicators.sh looks for setup.py again

Reverting a premature change.

* fix: readability and future-proof

* fix: more readability

* chore: bump delphi_utils to 0.3.25

* chore: bump covidcast-indicators to 0.3.56

* [create-pull-request] automated change

---------

Co-authored-by: Dmitry Shemetov <[email protected]>
Co-authored-by: minhkhul <[email protected]>
Co-authored-by: Nat DeFries <[email protected]>
Co-authored-by: minhkhul <[email protected]>
Co-authored-by: David Weber <[email protected]>
Co-authored-by: george <[email protected]>
Co-authored-by: Amaris Sim <[email protected]>
Co-authored-by: aysim319 <[email protected]>
Co-authored-by: Brian Clark <[email protected]>
Co-authored-by: Delphi Deploy Bot <[email protected]>
Co-authored-by: minhkhul <[email protected]>
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.

3 participants