-
Notifications
You must be signed in to change notification settings - Fork 14
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
Container config update for repose upgrade #96
Conversation
this needs some type of version check so that it will also work with the existing deployments in case we have to deploy the current version again. |
ef94d0b
to
33c1490
Compare
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.
LGTM
# After version 7.3.8, client_request_logging, connection_timeout, read-timeout & proxy-thread-pool attributes are depricated. | ||
# So here we had added check to add these attributes only for version smaller than 7.3.8. | ||
if (!@version.nil? && | ||
(majorVersion < 7 || |
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.
won't the normal string comparison work here? if version < '7.3.8'
. test ruby code shows it might be an option
version='7.3.8'
['1.2.3', '1.2', '7.0', '8.0', '7.4.1', '7.3.9', '7.2.1', '8.0.0'].each do |i|
if version < i
puts "#{ version } is less than #{i}"
else
puts "#{ version } is greater than #{i}"
end
end
OUTPUT:
7.3.8 is greater than 1.2.3
7.3.8 is greater than 1.2
7.3.8 is greater than 7.0
7.3.8 is less than 8.0
7.3.8 is less than 7.4.1
7.3.8 is less than 7.3.9
7.3.8 is greater than 7.2.1
7.3.8 is less than 8.0.0
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.
DOne the changes
While upgrading repose for blueflood-chef, we found out that some attributes of container deployment config are depricated after repose version 7.3.8. Attributes depricated are connection-timeout, read-timeout and client-request-logging. For connection-timeout and read-timeout, we found out that we can set these properties in http-pool-connection.cfg filter file present in repose code https://github.com/rackerlabs/repose/blob/master/repose-aggregator/artifacts/valve/src/config/filters/http-connection-pool.cfg.xml For read-timeout, we can set attribute http.socket-timeout as mentioned in its schema https://github.com/rackerlabs/repose/blob/f9a5ddcce0e2da4c1fcacd9f8f70d3dfb403c1bc/repose-aggregator/components/services/http-client-service/http-client-service-api/src/main/resources/META-INF/schema/config/http-connection-pool.xsd#L117 Documentation for this is present here https://www.openrepose.org/versions/8.10.0.0/services/http-connection-pool.html PR which mentioned attributes are depricated: rackerlabs/repose@a48bab7?diff=split&w=1 Client-request-logging attribute is not filtering out information. We can map client-request-logging attribute to HttpClient as mentioned below: https://github.com/rackerlabs/repose/blob/master/repose-aggregator/tests/functional-tests/src/integrationTest/groovy/features/core/proxy/ClientRequestLogging.groovy#L35
33c1490
to
78aef1a
Compare
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.
+1
While upgrading repose for blueflood-chef, we found out that some attributes of container deployment config are depricated after repose version 7.3.8. Attributes depricated are connection-timeout, read-timeout and client-request-logging.
For connection-timeout and read-timeout, we found out that we can set these properties in http-pool-connection.cfg filter file present in repose code
For read-timeout, we can set attribute http.socket-timeout as mentioned in its schema.
Documentation for this is present here
PR which mentioned attributes are depricated:
Depricate Container attributes
Client-request-logging attribute is not filtering out information. We can map client-request-logging attribute to HttpClient as mentioned here.