Skip to content
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

Add connection_string to interface and use in interface_microservice #1184

Merged
merged 6 commits into from
Apr 29, 2024

Conversation

jmthomas
Copy link
Member

@jmthomas jmthomas commented Apr 6, 2024

Still need to do Python but this is what I'm thinking

closes #1169

@jmthomas jmthomas requested a review from ryanmelt April 6, 2024 19:48
Copy link

codecov bot commented Apr 6, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 74.92%. Comparing base (89cb850) to head (130b90b).
Report is 23 commits behind head on main.

Files Patch % Lines
...c3/lib/openc3/interfaces/tcpip_server_interface.rb 36.36% 7 Missing ⚠️
...nc3/lib/openc3/interfaces/http_server_interface.rb 75.00% 1 Missing ⚠️
...lib/openc3/microservices/interface_microservice.rb 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1184      +/-   ##
==========================================
- Coverage   74.97%   74.92%   -0.06%     
==========================================
  Files         595      599       +4     
  Lines       43929    44200     +271     
  Branches      759      759              
==========================================
+ Hits        32936    33115     +179     
- Misses      10910    11001      +91     
- Partials       83       84       +1     
Flag Coverage Δ
frontend 55.37% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -31,6 +31,10 @@ def initialize(port = 80)
@request_queue = Queue.new
end

def connection_string
return "listening on #{@port}"
Copy link
Member

Choose a reason for hiding this comment

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

Needs hostname too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hostname isn't part of the connection parameters. It's just: @server = WEBrick::HTTPServer.new :Port => @port

Copy link
Member

Choose a reason for hiding this comment

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

It should be though. Should work the same as tcpip_server_interface.

@@ -627,7 +627,7 @@ def handle_connection_lost(err = nil, reconnect: true)
end

def connect
@logger.info "#{@interface.name}: Connecting ..."
@logger.info "#{@interface.name}: Connection #{@interface.connection_string}"
Copy link
Member

Choose a reason for hiding this comment

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

Connection doesn't read as nice as connecting... Maybe just Connecting: ?

Copy link
Member Author

Choose a reason for hiding this comment

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

How about Connect

@@ -119,6 +118,16 @@ def initialize(write_port,
@connected = false
end

def connection_string
if @write_port == @read_port
return "listening on #{@write_port} (R/W)"
Copy link
Member

Choose a reason for hiding this comment

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

Should include hostname

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

@jmthomas jmthomas merged commit 7a5490c into main Apr 29, 2024
23 of 24 checks passed
@jmthomas jmthomas deleted the connect_details branch April 29, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add additional connect details to interface connect errors
2 participants