Do not overwrite default_queue_type with undefined
#11457
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukebakken
force-pushed
the
lukebakken/vesc-1108-dont-overwrite-default_queue_type
branch
from
582d3d5 to
b1c8a4b
Compare
|
@michaelklishin this PR will prevent |
lukebakken
force-pushed
the
lukebakken/vesc-1108-dont-overwrite-default_queue_type
branch
3 times, most recently
from
d986ead to
d6774fe
Compare
michaelklishin
approved these changes
Jun 14, 2024
lukebakken
force-pushed
the
lukebakken/vesc-1108-dont-overwrite-default_queue_type
branch
from
636152e to
2739c35
Compare
|
Re-requesting reviews from @michaelklishin and @ansd since I was out this week and just want a double-check before merging. Thanks gents. |
Importing a definitions file with no `default_queue_type` metadata for a vhost will result in that vhosts value being set to `undefined`. Once set to a non-`undefined` value, this PR prevents `default_queue_type` from being set back to `undefined`
michaelklishin
force-pushed
the
lukebakken/vesc-1108-dont-overwrite-default_queue_type
branch
from
2739c35 to
1cf5477
Compare
|
The forced push was a rebase. |
michaelklishin
approved these changes
Jun 21, 2024
lukebakken
deleted the
lukebakken/vesc-1108-dont-overwrite-default_queue_type
branch
michaelklishin
added a commit
that referenced
this pull request
Jun 22, 2024
In case 16, an await_condition/2 condition was
not correctly matching the error. As a result,
the function proceeded to the assertion step
earlier than it should have, failing with
an obscure function_clause.
This was because an {error, Context} clause
was not correct.
In addition to fixing it, this change adds a
catch-all clause and verifies the loaded
tagged virtual host before running any assertions
on it.
If the virtual host was not imported, case 16
will now fail with a specific CT log message.
References #11457 because the changes there
exposed this behavior in CI.
michaelklishin
added a commit
that referenced
this pull request
Jun 22, 2024
In case 16, an await_condition/2 condition was
not correctly matching the error. As a result,
the function proceeded to the assertion step
earlier than it should have, failing with
an obscure function_clause.
This was because an {error, Context} clause
was not correct.
In addition to fixing it, this change adds a
catch-all clause and verifies the loaded
tagged virtual host before running any assertions
on it.
If the virtual host was not imported, case 16
will now fail with a specific CT log message.
References #11457 because the changes there
exposed this behavior in CI.
(cherry picked from commit f515282)
michaelklishin
added a commit
that referenced
this pull request
Jun 22, 2024
In case 16, an await_condition/2 condition was
not correctly matching the error. As a result,
the function proceeded to the assertion step
earlier than it should have, failing with
an obscure function_clause.
This was because an {error, Context} clause
was not correct.
In addition to fixing it, this change adds a
catch-all clause and verifies the loaded
tagged virtual host before running any assertions
on it.
If the virtual host was not imported, case 16
will now fail with a specific CT log message.
References #11457 because the changes there
exposed this behavior in CI.
michaelklishin
added a commit
that referenced
this pull request
Jun 22, 2024
The queue type argument won't always be a binary, for example, when a virtual host is created. As such, the validation code should accept at least atoms in addition to binaries. While at it, improve logging and error reporting when DQT validation fails, and while at it, make the definition import tests focussed on virtual host a bit more robust.
Merged
michaelklishin
added a commit
that referenced
this pull request
Jun 22, 2024
…post-11457 Follow-up to #11457
mergify bot
pushed a commit
that referenced
this pull request
Jun 22, 2024
The queue type argument won't always be a binary, for example, when a virtual host is created. As such, the validation code should accept at least atoms in addition to binaries. While at it, improve logging and error reporting when DQT validation fails, and while at it, make the definition import tests focussed on virtual host a bit more robust. (cherry picked from commit 1e577a8) # Conflicts: # deps/rabbit/src/rabbit_vhost.erl
mergify bot
pushed a commit
that referenced
this pull request
Jun 24, 2024
at validation time. DQT = default queue type. When a client provides no queue type, validation should take the defaults (virtual host, global, and the last resort fallback) into account instead of considering the type to be "undefined". References #11457 ##11528 (cherry picked from commit f3b7a34) # Conflicts: # deps/rabbit/src/rabbit_queue_type.erl # deps/rabbit/src/rabbit_vhost.erl
michaelklishin
added a commit
that referenced
this pull request
Jun 24, 2024
at validation time. DQT = default queue type. When a client provides no queue type, validation should take the defaults (virtual host, global, and the last resort fallback) into account instead of considering the type to be "undefined". References #11457 ##11528 (cherry picked from commit f3b7a34) # Conflicts: # deps/rabbit/src/rabbit_queue_type.erl # deps/rabbit/src/rabbit_vhost.erl
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Importing a definitions file with no
default_queue_typemetadata for a vhost will result in that vhosts value being set toundefined.Once set to a non-
undefinedvalue, this PR preventsdefault_queue_typefrom being set back toundefined