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

chore: don't download draft releases #771

Merged
merged 2 commits into from
Sep 26, 2024
Merged

Conversation

vjousse
Copy link
Collaborator

@vjousse vjousse commented Sep 26, 2024

🔧 Problem

When deploying on scalingo we download all releases without checking if it's a draft one, it may cause problems if we want to use the draft functionality of releases to use git-cliff for example #768.

🍰 Solution

Filter out releases that are draft.

🏝️ How to test

The deploy to scalingo of this PR should download 11 releases and not more (we currently have 11 releases and some drafts).

@vjousse vjousse self-assigned this Sep 26, 2024
@vjousse vjousse marked this pull request as ready for review September 26, 2024 13:00
@vjousse vjousse changed the title fix: don't download draft releases chore: don't download draft releases Sep 26, 2024
@vjousse vjousse requested review from n1k0 and ccomb September 26, 2024 13:36
Copy link
Member

@n1k0 n1k0 left a comment

Choose a reason for hiding this comment

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

Will the logged error show up in Sentry? I suppose so! All good 👍

with open(local_filename, "wb") as f:
shutil.copyfileobj(r.raw, f)
else:
logger.error(f"Status: {r.status_code}, body: {r.text}")
Copy link
Member

Choose a reason for hiding this comment

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

🙏

continue

for asset in release.assets:
if "-dist.tar.gz" in asset.browser_download_url:
file_path = download_file(
asset.browser_download_url, args.destination_directory
)
if not file_path:
logger.error(
f"Error downloading {asset.browser_download_url}, skipping."
Copy link
Member

Choose a reason for hiding this comment

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

👌

@vjousse
Copy link
Collaborator Author

vjousse commented Sep 26, 2024

Will the logged error show up in Sentry? I suppose so! All good 👍

As far as I know, sentry as never been enabled on the python side 😬

@n1k0
Copy link
Member

n1k0 commented Sep 26, 2024

As far as I know, sentry as never been enabled on the python side 😬

Argh, you're right! Would be nice to be notified when release archives are unavailable or things like that… but I suspect we'd get many errors as it would depend on GH infrastructure availability, which has proven slightly unreliable over time.

@vjousse vjousse merged commit 1f68b23 into master Sep 26, 2024
7 checks passed
@vjousse vjousse deleted the fix/dont-dowload-draft-releases branch September 26, 2024 15:00
vjousse pushed a commit that referenced this pull request Oct 10, 2024
## [2.4.0](https://github.com/MTES-MCT/ecobalyse/compare/v2.3.0..v2.4.0)
(2024-10-10)



### 🚀 Features

- Introduce first version of object interface
([#756](#756))

### 🪲 Bug Fixes

- Sync food ([#759](#759))
- Don't hide version information on staging
([#778](#778))
- Reset physical durablility in regulatory mode
([#786](#786))
- *(api,food)* Nullable fields weren't nullable anymore.
([#789](#789))

### 🚜 Refactor

- Small textile explorer improvements
([#773](#773))

### ⚙️ Miscellaneous Tasks

- Don't download draft releases
([#771](#771))
- Remove `airTransportRatio` from examples
([#785](#785))
- Cleanup package-lock.json.
([#787](#787))
- Use builtin python action cache for pipenv
([#796](#796))
- Improve changelog by using `git-cliff`
([#768](#768))

<!-- generated by git-cliff -->

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants