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

[Content] Large content files should be removed from repo #35

Closed
2 of 3 tasks
frgfm opened this issue Dec 24, 2020 · 6 comments · Fixed by #81
Closed
2 of 3 tasks

[Content] Large content files should be removed from repo #35

frgfm opened this issue Dec 24, 2020 · 6 comments · Fixed by #81
Labels
help wanted Extra attention is needed topic: core platform features type: enhancement New feature or request
Milestone

Comments

@frgfm
Copy link
Member

frgfm commented Dec 24, 2020

Simply put, graphics assets could indeed be stored in the git history as they have a very limited size (event though we could avoid it). However, content files like the French geojson or wildfire history data should be moved to release attachments or on cloud storage to preserve git history sanity.

They could be downloaded when the app starts and cached.

@frgfm frgfm added this to the 0.1.2 milestone Dec 24, 2020
@pechouc
Copy link
Collaborator

pechouc commented Feb 4, 2021

Hello @frgfm!

Sorry for answering this issue so late, we have indeed added to our to-do list the removal of local data files from the repository. These are:

  • cameras.csv - Everything is ready to tackle this one thanks to the get_sites function of the API client (thanks a lot for the work done on that side by the way!). We are going to implement it soon with @abdi-adel 😊

  • departments.geojson - For this one, we cannot directly use the file available online since the website was recently down for several weeks in a row according to @chloeskt. Apparently, in the pyro-risk repository, it was attached to a release. So, what do you think is best between fetching the file from there or having it released in the pyro-platform repository too? If the first option is fine, we can implement it very shortly.

  • historic_fires.csv - There might be a bit more material for discussion here but I must admit I have no idea what would be the best practice.

    • On the one hand, we haven't planned to update this list of past fires anytime soon and therefore, the file is probably bound to remain static for a while (which would justify attaching it to a release I guess).
    • But in a way, it would make sense to store it in the database because (i) it could be useful not only to the platform team and (ii) this would be the occasion to script the logic that allowed us to preprocess the raw data found on data.gouv, so that we would be able to refresh it every time the dataset is updated.

@frgfm
Copy link
Member Author

frgfm commented Feb 5, 2021

No worries, we all had our hands busy!

  • departments.geojson: for now, yes it would be a good idea to fetch it from the release when you start the app (if it's not already downloaded)
  • historic_fires.csv: this one will in the end be available through the API as well. The key here would perhaps be to add some previous wildfires thoroughly in the API DB

@pechouc
Copy link
Collaborator

pechouc commented Feb 6, 2021

Thanks for your answer 🙏

I have made a quick test in a notebook to check whether we can fetch the .geojson file from the release and it works just fine. We will implement it very soon in the app itself.

As for historic_fires.csv, it would be great to eventually call it through the API indeed! Should we open a dedicated issue in the pyro-api repository for instance? So that we can show you in more details the preprocessing logic we have established and what the raw data files look like.

@frgfm
Copy link
Member Author

frgfm commented Feb 6, 2021

Cool for the geojson!

Yes, I just checked there is no available method in the API client so far (but I doubt we'll need to implement a route in the API itself)

@pechouc
Copy link
Collaborator

pechouc commented Feb 15, 2021

NB: I opened this issue in the Pyro-API repository to discuss the addition of historic fires to the database 🙂

@frgfm frgfm added the help wanted Extra attention is needed label Feb 17, 2021
@frgfm
Copy link
Member Author

frgfm commented Jul 4, 2022

We discussed where it could be stored, but I might have a better question @pyronear/front-end : what is the feature related to historic past wildfires? Do we really need it?

This is a monitoring platform so I don't see the value of displaying static information. If we are to display past events happening in their region, we need to fetch it through the API. But I'm starting to wonder whether these historic fires shouldn't be removed, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed topic: core platform features type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants