-
Notifications
You must be signed in to change notification settings - Fork 168
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
Workaround issue when initiating sync on a copied realm file #4878
Changes from all commits
1ca0865
a00e55a
0e9713a
c38b169
adb9155
2c942d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -811,8 +811,10 @@ void ClientHistoryImpl::update_sync_progress(const SyncProgress& progress, | |
version_type(root.get_as_ref_or_tagged(s_progress_download_client_version_iip).get_as_int())); | ||
REALM_ASSERT(progress.upload.client_version >= | ||
version_type(root.get_as_ref_or_tagged(s_progress_upload_client_version_iip).get_as_int())); | ||
REALM_ASSERT(progress.upload.last_integrated_server_version >= | ||
version_type(root.get_as_ref_or_tagged(s_progress_upload_server_version_iip).get_as_int())); | ||
if (progress.upload.last_integrated_server_version > 0) { | ||
REALM_ASSERT(progress.upload.last_integrated_server_version >= | ||
version_type(root.get_as_ref_or_tagged(s_progress_upload_server_version_iip).get_as_int())); | ||
} | ||
|
||
auto uploaded_bytes = std::uint_fast64_t(root.get_as_ref_or_tagged(s_progress_uploaded_bytes_iip).get_as_int()); | ||
auto previous_upload_client_version = | ||
|
@@ -829,8 +831,10 @@ void ClientHistoryImpl::update_sync_progress(const SyncProgress& progress, | |
RefOrTagged::make_tagged(progress.latest_server_version.salt)); // Throws | ||
root.set(s_progress_upload_client_version_iip, | ||
RefOrTagged::make_tagged(progress.upload.client_version)); // Throws | ||
root.set(s_progress_upload_server_version_iip, | ||
RefOrTagged::make_tagged(progress.upload.last_integrated_server_version)); // Throws | ||
if (progress.upload.last_integrated_server_version > 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the current version of s_progres_upload_server_version_iip is zero, would this be a noop without this conditional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand the question. If current value is zero and progress value is zero then it is obviously a noop. The test just makes sure that we will not overwrite a non-zero value with a zero. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you are answering my question without understanding it 😄 . We were changing this from unconditionally setting a value to conditionally setting a value, and I wanted to verify the preconditions. Can you add a comment about why we're conditionally setting this here? |
||
root.set(s_progress_upload_server_version_iip, | ||
RefOrTagged::make_tagged(progress.upload.last_integrated_server_version)); // Throws | ||
} | ||
if (downloadable_bytes) { | ||
root.set(s_progress_downloadable_bytes_iip, | ||
RefOrTagged::make_tagged(*downloadable_bytes)); // Throws | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#!/usr/bin/env bash | ||
|
||
PROJECT_DIR=$(git rev-parse --show-toplevel) | ||
MDBREALM_TEST_SERVER_TAG=$(grep MDBREALM_TEST_SERVER_TAG ${PROJECT_DIR}/dependencies.list |cut -f 2 -d=) | ||
docker run --rm -p 9090:9090 -v ~/.aws/credentials:/root/.aws/credentials -it docker.pkg.github.com/realm/ci/mongodb-realm-test-server:${MDBREALM_TEST_SERVER_TAG} |
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.
Can we just remove these suffixes? What value are they adding?
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.
They are adding the value that It is build into the name which version it can handle. I have to have multiple versions around so that I can open whatever realm file I get from a customer.
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.
Ah, if it's part of your workflow then okay. I wish this were configurable. We use these tools outside of your debugging processes - e.g. the cloud team depends on them. @tkaye407 , will you need to update anything if we rename realm2json like this?
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 currently just pin to a specific version (though we probably should be able to pick up the latest merge), but once this goes in I can update the version / name of the executable that we link to in our evergreen config. Thanks for pointing out