8000 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 terms 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