Remove unused properties#426
Merged
mattklein123 merged 2 commits intoenvoyproxy:masterfrom Feb 6, 2017
Merged
Conversation
mattklein123
approved these changes
Feb 4, 2017
Member
There was a problem hiding this comment.
since this is not used at all, can you fix ctor and remove this param completely?
Contributor
Author
There was a problem hiding this comment.
Fine w/me will change
Reflejo
force-pushed
the
remove-unused-properties
branch
2 times, most recently
from
ac5dcbc to
39baae5
Compare
The MOCK_METHOD* macros from gtest are not including the override attribute and it's triggering warnings. This change includes `-Wno-inconsistent-missing-override` to that target.
Reflejo
force-pushed
the
remove-unused-properties
branch
from
39baae5 to
fc60626
Compare
rshriram
pushed a commit
to rshriram/envoy
that referenced
this pull request
Oct 30, 2018
Automatic merge from submit-queue. Introduce new attribute connection.mtls **What this PR does / why we need it**: Introduce new mixer attribute "connection.mtls" which indicates whether connection is mutual TLS enabled. Add virtual function bool IsMutualTlsEnabledConnection() into http/check_data.h and tcp/check_data.h, and call this function inside AttributesBuilder::ExtractCheckAttributes(). Add another virtual function bool GetDestinationIpPort(std::string* ip, int* port) to report destination IP and port from HTTP filter. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes envoyproxy#426 **Special notes for your reviewer**: **Release note**: ```release-note ```
wolfguoliang
pushed a commit
to wolfguoliang/envoy
that referenced
this pull request
Jan 23, 2021
mathetake
pushed a commit
that referenced
this pull request
Mar 3, 2026
**Commit Message**
* adding e2e test making sure that we can use a tool and return the
proper response from the LLM
This PR also contains a few fixes that allow this test to work
* added validateToolCallID as its needed for the bedrock message or it
fails
* Validate and cast the openai content value into bedrock content block
- before we assumed it was always a string but it can also be an array
* Merge the content blocks if one has text and the other has toolcall
but no text - bedrockResp.Output.Message.Content is not 1:1 with openai
choice.Message.Content.. we get the result in chunks that should be
merged, for example the text and it's toolconfig were in seperate
elements but openai expects them to be in the same
* in openAIMessageToBedrockMessageRoleAssistant we assume that either
there is a refusal or a content text, but we actually dont pass in a
text. this was causing an error as the length of the array was set to 1
so the first was empty and there must be a key specified in each content
element.
note: i think this is another bug that im trying to look into/create
unit test for, but since there is already a lot in this PR, maybe its
best to follow up with that one the next one. basically, the assistant
openai param message's content doesn't seem to be translating to this
method, and we have no content. this doesn't prevent anything else from
working though, we are just missing a text field like
`{"content":[{"text":"Certainly! I can help you get the weather
information for New York City. To do that, I'll use the available
weather tool. Let me fetch that information for you right away."}`
the rests are tests
---------
Signed-off-by: Alexa Griffith <agriffith96@gmail.com>
Signed-off-by: Alexa Griffith <agriffith96@gmail.com>
Signed-off-by: Dan Sun <dsun20@bloomberg.net>
Co-authored-by: Dan Sun <dsun20@bloomberg.net>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Not sure if this was intended but they are unused.
Also added a fix for some compilation warnings on the test target when compiling with clang.