10000 fix it! by jsteemann · Pull Request #10198 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

fix it! #10198

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our term 10000 s of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 10, 2019
Merged

fix it! #10198

merged 1 commit into from
Oct 10, 2019

Conversation

jsteemann
Copy link
Contributor

Scope & Purpose

This bug is about:

conspire = function() {
  let theories = [
    "docker", "ubuntu", "k8s", "json", "vst", "c#", "javascript", "collectd", 
    "sleep", "satellite collections", "gyp", "lto", "macos", "smart joins",  
    "el cheapo", "v8", "salat", "windows", "transactions", "oskar", "fuerte",
    "ccache", "libmusl"
  ];
  let conspiracy = {};
  while (Object.keys(conspiracy).length < 3) {
    conspiracy[theories[Math.floor(Math.random() * theories.length)]] = true;
  }
  return Object.keys(conspiracy);
};
print("It is caused by:", conspire().join(" + "));
  • Bug-Fix for devel
  • The behavior in this PR can be (and was) manually tested (support / qa / customers can test it)

Testing & Verification

This change is a trivial rework / code cleanup without any test coverage.

https://172.16.10.101/view/PR/job/arangodb-matrix-pr/6647/

@jsteemann jsteemann added this to the devel milestone Oct 8, 2019
@jsteemann jsteemann requested a review from dothebart October 8, 2019 21:41
graetzer
graetzer approved these changes Oct 8, 2019
View reviewed changes
arangod/RestHandler/RestVocbaseBaseHandler.cpp
@@ -487,7 +488,7 @@ void RestVocbaseBaseHandler::generateTransactionError(std::string const& collect
return;

case TRI_ERROR_ARANGO_CONFLICT:
if (result.buffer != nullptr) {
if (result.buffer != nullptr && !result.slice().isNone()) {
// This case happens if we come via the generateTransactionError that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this consistent with the behavior in the other crud methods now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. But if the slice is None, we will not be able to serialize it to JSON later on.

@jsteemann jsteemann merged commit 69dc506 into devel Oct 10, 2019
@jsteemann jsteemann deleted the bug-fix/fix-random-willi-bug branch October 10, 2019 13:34
ObiWahn added a commit that referenced this pull request Oct 11, 2019
@ObiWahn
Merge branch 'devel' of https://github.com/arangodb/arangodb into fea…
7f67ae6 
…ture/one-shard-clean-up-2

* 'devel' of https://github.com/arangodb/arangodb:
  Bug fix/improve stringutils performance (#10208)
  add option to talk to the SUT using VST (#10217)
  Doc - Added "log-output" example (#10207)
  fix it! (#10198)
  add missing include
  Bug fix/fix simple example dep proxy skip some regression test (#10213)
  fixed ui behaviour when replacing a foxx app (#9719)
  [devel] Fix document search (Ctrl+F/Cmd+F) (#10216)
  Convert many uses of ClusterComm to Fuerte (#10154)
  Remove invokeOnAllElements (#10212)
  AQL Subquery: MultiDependencyRowFetcher (#10101)
  Bug fix/fix remote executor races (#10206)
  fix several inefficiencies in Store (#10189)
  Deprecate rocksdb.max-write-buffer-number startup option (#9654)
  fix arangosh with vst
ObiWahn added a commit that referenced this pull request Oct 14, 2019
@ObiWahn
Merge branch 'devel' of https://github.com/arangodb/arangodb into fea…
a15f460 
…ture/cpp17

* 'devel' of https://github.com/arangodb/arangodb: (46 commits)
  use automatic parsing so we can seamlessly switch HTTP / VST. (#10241)
  fix handling of VST urls in test (#10240)
  if we answer a head request, we mustn't create a body for errors (#10227)
  Move containers into their own folder and namespace. (#10244)
  when changing between leader and follower re-attempt connecting to circumvent OS resource shortages (#10243)
  Fix content-type for JS APIs (#10225)
  Feature/one shard clean up 2 (#10238)
  Fix dbserver locking and releasing for hotbackup. (#10177)
  Improve timings for hotbackup locking. (#10229)
  unintentionally broke something. fixed...
  throw parse error if we fail to parse the json document (#10226)
  Bug fix/improve stringutils performance (#10208)
  add option to talk to the SUT using VST (#10217)
  Doc - Added "log-output" example (#10207)
  fix it! (#10198)
  add missing include
  Bug fix/fix simple example dep proxy skip some regression test (#10213)
  fixed ui behaviour when replacing a foxx app (#9719)
  [devel] Fix document search (Ctrl+F/Cmd+F) (#10216)
  Convert many uses of ClusterComm to Fuerte (#10154)
  ...
ObiWahn added a commit that referenced this pull request Oct 14, 2019
@ObiWahn
Merge remote-tracking branch 'origin/feature/cpp17' into feature/cpp1…
7f8dcd9 
…7-remove-mac-hack

* origin/feature/cpp17:
  use automatic parsing so we can seamlessly switch HTTP / VST. (#10241)
  fix handling of VST urls in test (#10240)
  if we answer a head request, we mustn't create a body for errors (#10227)
  Move containers into their own folder and namespace. (#10244)
  when changing between leader and follower re-attempt connecting to circumvent OS resource shortages (#10243)
  Fix content-type for JS APIs (#10225)
  Feature/one shard clean up 2 (#10238)
  Fix dbserver locking and releasing for hotbackup. (#10177)
  Improve timings for hotbackup locking. (#10229)
  unintentionally broke something. fixed...
  throw parse error if we fail to parse the json document (#10226)
  Bug fix/improve stringutils performance (#10208)
  add option to talk to the SUT using VST (#10217)
  Doc - Added "log-output" example (#10207)
  fix it! (#10198)
  add missing include
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@graetzer graetzer graetzer approved these changes

@dothebart dothebart dothebart approved these changes

Assignees
No one assigned
Labels
9 Ready to Merge
Projects
None yet
Milestone
devel
Development

Successfully merging this pull request may close these issues.
3 participants
@jsteemann @graetzer @dothebart
0