-
Notifications
You must be signed in to change notification settings - Fork 9
Push v2 integration #262
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: master
Are you sure you want to change the base?
Push v2 integration #262
Conversation
- attempts of failing chunks =- init for sync_project
- fixing update chunks for files with the same checksum - added minimal version of sync command
Introduce mapping based on chunk_id (generated by client) and server_chunk_id generated by server.
upload_job = None | ||
length = 1 | ||
try: | ||
def on_progress(increment, push_job): |
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.
As a user I would probably like to see individual steps, each with progress bar rather than one global adaptive bar.
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, we can handle this progress bar is tricky to do... let me take a look at this after finish of logic
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.
It's not possible to create addaptive bar in click
version: Optional[str] = None | ||
# some functions (MerginProject.compare_file_sets) are adding history dict to the change from project info | ||
history: Optional[dict] = None | ||
# some functions (MerginProject.compare_file_sets) are adding location dict to the change from project info |
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.
This is probably a bug, as it comes in server response but it should not.
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 here, as client is using some things from this dictinary.
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.
sorry, I meant location
key
with open(self.file_path, "rb") as file_handle: | ||
file_handle.seek(self.chunk_index * UPLOAD_CHUNK_SIZE) | ||
data = file_handle.read(UPLOAD_CHUNK_SIZE) | ||
for chunk_index, chunk_id in enumerate(change.chunks): |
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.
Do we want to rely here on chunks? Maybe we should take files and split to chunks here and not before.
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 need chunk index and chunk id to send in v1 endpoints to push. It's coming from MerginProject.get_push_changes huge logic over chunks, where not just files are splitted, but also diffs. Not sure if we want to move it here.
- +v1 is raising error just from push start (push_project_async), because there are not chunks caches
…r chunks cache on v1 supported server (#276)
Added ➕
Updated tests:
❓
❗
Chunks cache
Logging following to logs
Retry of sync_project
Workflow
