-
Notifications
You must be signed in to change notification settings - Fork 0
export network notification header #110
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
base: main
Are you sure you want to change the base?
Conversation
static final String HEADER_INDEXATION_STATUS = "indexation_status"; | ||
static final String HEADER_COMPUTATION_TYPE = "computationType"; | ||
static final String HEADER_RESULT_UUID = "resultUuid"; | ||
static final String HEADER_FILE_NAME = "fileName"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure fileName and format should be there. Maybe store them as s3 metadata ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if in the future we want to be able to download the file after the notification is done, we need to store these data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t really understand your change request reason do you want me to remove those two parameters from here?
In that case, how would the frontend get this information?
And regarding your S3 metadata solution, do you want me to implement it now, or is it something we should keep for later?
And could you clarify what exactly I should change here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I mean that it's not a good system that the fileName and format goes from network-conversion-server to the front and then back to network-conversion-server. The API should work only with the exportUuid and the fileName and format should be store in S3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do it in this PR yes. To be able in the future to download a case out of the notification system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if we export the same node, using the same format and file name, one right after the other?
IMO, we should generate an exportUuid (similar to a command ID) at the frontend and include it in the request.
The debug file currently uses (nodeUuid, computationType) as a functional key to identify notifications, since only one computation type can occur at a given moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have 2 simultaneous exports on the same node, it gonna work with the exportUuid that is unique. Not sure we'll a problem nope ?
|
No description provided.