Comments (7)
Agreed, I would have a simple suggestion for the key:
- either a concat of device_id and timestamp
- or the better option, the SHA256 hash of the content file
Fully agree that the update should only be made if the upload is successful!
from pyro-api.
We already do the bucket_key upload only if the upload was successfful.
entry = await check_for_media_existence(media_id, current_device.id)
bucket_key = str(hash(datetime.utcnow()))
upload_success = await bucket_service.upload_file(bucket_key=bucket_key,
file_binary=file.file)
if upload_success is False:
raise HTTPException(
status_code=500,
detail="The upload did not succeed"
)
entry = dict(**entry)
entry["bucket_key"] = bucket_key
return await crud.update_entry(media, MediaCreation(**entry), media_id)
Agreed for the hashing part.
My point is a bit different, if you read the code above, you will see we don't check there isn't already an uploaded image for a media. If we run the route three times for one unique media, we will have three different images in the CSP.
Don't know exactly what would be the desired behavior. Overwriting?
from pyro-api.
Mmmh, if we have only the content and not the timestamp, I guess we can check if the media content already exists on the bucket?
Actually, this might be a more complex problem: the upload can succeed (ie media content is valid and in the bucket) and we try to overwrite it, but the upload can also fail explicitly (connection issue or something similar) or silently (data corruption during transfer).
So what we could do when we upload a media:
- check if there is a bucket key existing in the table
- if there is, we need to decide whether overwrite is allowed or if we do nothing
- if there isn't, we compute the hashed bucket key
- check if the bucket has a file named the same way
- if there is, we again need to make the same decision
- if there isn't, we upload the file
- once uploaded, we could also try to dl the file, and hash the downloaded version to verify data integrity
As a first step, we could "do nothing" when the file is already in the bucket.
from pyro-api.
Thinking back about that issue, now that data integrity is fully deployed:
- unicity / hashing scheme is currently based solely on the content of the file (no timestamp). This means that if several users try to upload the exact same file (let's say they used the same publicly available image), two things will happen: it will create to abstract media objects pointing to the same bucket key, and only the first uploaded file will remain (it is on purpose to save data transfer and storage).
- if we keep the current hashing scheme, when a media object is deleted, we can't delete the actual bucket file, because we need to check whether another active media uses this content.
- same problem comes when we overwrite the bucket key of a media file: the previous content is kept for now
So my suggestion is simple: add timestamp in the hash.
Inconvenients:
- breaking change: we will have to reset the DB entirely, because current bucket key will not pass the proposed integrity check (because of the hashing scheme modification)
- potential storage & transfer overhead: in production this will most likely never happen, but when uploading images from the web, we might have some overhead
Advantages:
- no more question about whether we can delete a media, so overwriting & deletion of a media bucket key should lead to the actual deletion of the bucket file.
Happy to open a PR if you think we should move forward with this. What do you think?
from pyro-api.
Any opinion on the topic @jeanpasquier75 @martin1tab @fe51 ?
from pyro-api.
This can become an issue once we deploy the API more widely. Any thoughts @jeanpasquier75 @florianriche @fe51 @martin1tab ?
from pyro-api.
Moving this to a later release since we need to decide first what to do in this specific situation (and it doesn't sound critical before receiving any live alerts)
from pyro-api.
Related Issues (20)
- [docs] Add a tutorial/walkthrough to deploy everything on your own HOT 4
- Running the docker compose locally does not start the API correctly HOT 6
- [admin scope issue][media] HOT 1
- [Ongoing Alerts] Specify the limit size of the response in API documentation HOT 1
- Code quality checks failing for main branch HOT 1
- Creating a group with an existing name triggers an internal error
- No timeout in api client ?
- Allow to remove max request response size HOT 2
- [media] Media deletion crashes when no bucket key is set
- [installations] Internal error when specifying datetime in payload
- How webhooks work HOT 2
- Tests failing often due httpbin.org timeout HOT 1
- Clarify when (and how) users take events into account HOT 6
- Document the process to update the db diagram HOT 1
- Fetch routes do not limit number of items for non-admin users HOT 1
- Grouping alerts HOT 2
- Access api documentation HOT 1
- Add a proxy url for the s3 media fetch HOT 2
- Set proper Continuous Deployement HOT 1
- Clarifying data/state tables from event tables HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from pyro-api.