Skip to content

Conversation

suvanbanerjee
Copy link
Collaborator

No description provided.

@suvanbanerjee suvanbanerjee requested a review from braddf June 24, 2025 18:59
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@braddf the main focus for this pr should be this file

@suvanbanerjee
Copy link
Collaborator Author

@braddf, apart from the file I said on, please feel free to skim through the rest since most of the files are auto-generated by fastapi-template. The testing framework is set up, but there aren’t any real tests yet; I plan to add them later this week for the S3 fetch. The same goes for the README.md.

Copy link
Contributor

@braddf braddf left a comment

Choose a reason for hiding this comment

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

Just leaving these couple of comments from before our chat this morning, let me know when the update is ready for review! 👍

Copy link
Contributor

@braddf braddf left a comment

Choose a reason for hiding this comment

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

Quite a few comments, most should be easy changes, and let me know if you are uncertain about anything. I have been fairly strict, as I think what you've done merits a higher level of attention to detail and design standards 👍

@@ -1,19 +0,0 @@
name: Non-Default Branch Push CI (Python)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you reinstate these two default workflows? So that this repo does the same things as others 👍


class InterceptHandler(logging.Handler):
"""
Default handler from examples in loguru documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't used loguru before, but will give it a go!

"cloudcasting_backend/static/layers" # Base directory for all GeoTIFF output
)
# Bounding box for cropping the GeoTIFF output [lon_min, lat_min, lon_max, lat_max]
GEOTIFF_BBOX = [-19.0, 42.0, 15.0, 65.0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the same for both MET-10 and MET-11?

dst.write(data, 1)
log.success(f"Successfully saved GeoTIFF to {filename}")
except Exception as e:
log.opt(exception=e).error(f"Failed to save GeoTIFF file: {filename}")
Copy link
Contributor

Choose a reason for hiding this comment

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

So actually, I know I said Sentry works out of the box, but if you're catching and logging exceptions like here, then you can also just fire them into Sentry using the functions in the lib, which collects them nicely for reviewing

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