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.

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

Upgrade fedora to version 40 in GHA CI #1502

Merged
merged 3 commits into from
May 22, 2024
Merged

Upgrade fedora to version 40 in GHA CI #1502

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a1da2d
CI: upgrade fedora platform to version 40
kgiusti May 20, 2024
e77bea4
fixup: correct python linter errors
kgiusti May 20, 2024
2fec874
fixup: fat-fingered the containerTag value
kgiusti May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ jobs:
matrix:
os: [ubuntu-22.04]
container: ['fedora']
containerTag: ['39']
containerTag: ['40']
cc: [gcc]
cxx: [g++]
python: [python3.11]
Expand Down Expand Up @@ -583,7 +583,7 @@ jobs:
# # unittest coverage
# - os: ubuntu-22.04
# container: 'fedora'
# containerTag: 39
# containerTag: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Although this has line has been commented out, containerTag must be 40 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

woopsie - good catch. Fix upcoming.

# cc: gcc
# cxx: g++
# python: python3.12
Expand All @@ -597,7 +597,7 @@ jobs:
# # clang
#- os: ubuntu-22.04
# container: 'fedora'
# containerTag: 39
# containerTag: 40
# cc: clang
# cxx: clang++
# python: python3.11
Expand All @@ -608,7 +608,7 @@ jobs:
# shards: 2
#- os: ubuntu-22.04
# container: 'fedora'
# containerTag: 39
# containerTag: 40
# cc: clang
# cxx: clang++
# python: python3.11
Expand Down
3 changes: 1 addition & 2 deletions python/skupper_router/management/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,7 @@ def query(self, type=None, attribute_names=None, offset=None, count=None):
if count == len_response_results:
break

if count - len_response_results < request_count:
request_count = count - len_response_results
request_count = min(request_count, count - len_response_results)

offset += request_count

Expand Down
1 change: 1 addition & 0 deletions python/skupper_router_internal/management/qdrouter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def validate_add(
super(QdSchema, self).validate_add(attributes, entities)
entities.append(attributes)
router_mode = router_id = listener_connector_role = listener_role = None
list_conn_entity = None
for e in entities:
short_type = self.short_name(e['type'])
if short_type == "router":
Expand Down
4 changes: 1 addition & 3 deletions python/skupper_router_internal/tools/display.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,7 @@ def table(self, title, heads, rows):
width = len(head)
for row in rows:
text = UNICODE(row[col])
cellWidth = len(text)
if cellWidth > width:
width = cellWidth
width = max(width, len(text))
colWidth.append(width + self.tableSpacing)
line = line + head
if col < len(heads) - 1:
Expand Down
8 changes: 4 additions & 4 deletions requirements-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ websockets>=9.1
psutil

# For running the python-checker tests
hacking==6.1.0
flake8==6.1.0
pylint==3.0.3
mypy==1.8.0
hacking>=6.1.0
flake8>=6.1.0
pylint>=3.0.3
mypy>=1.8.0
Comment on lines +45 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you realize what this means (different people with different versions of these tools will get different linter warnings), so no need for me to point this out.

Besides, as things now stand, it is already true that sometimes the os versions and sometimes the pypi venv versions of these tools are used, depending on how one sets up their dev environment, so this is not actually something entirely new. What's now new is that this will be also seen in CI (new ci failures on python liner checks because pypi packages were updated, without any code changes in router)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - this change is a bit heavy handed. I had to advance pylint (I think it was pylint) as the 3.0.3 version was actually failing on F40 (some type assertion).

Having newer linter versions catch new errors is a good thing... probably. If it introduces lots of noise then yeah we should set upper version bounds as necessary.

8 changes: 5 additions & 3 deletions tests/TCP_echo_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ def run(self):
total_sent = 0
total_rcvd = 0

payload_out = []
out_list_idx = 0 # current _out array being sent
out_byte_idx = 0 # next-to-send in current array
out_ready_to_send = False

if self.count > 0 and self.size > 0:
# outbound payload only if count and size both greater than zero
payload_out = []
out_list_idx = 0 # current _out array being sent
out_byte_idx = 0 # next-to-send in current array
out_ready_to_send = True
# Generate unique content for each message so you can tell where the message
# or fragment belongs in the whole stream. Chunks look like:
Expand Down
1 change: 0 additions & 1 deletion tests/TCP_echo_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def connection_made(self, transport):
if self.echo_server.close_on_conn:
self.logger.log(f' {self.name}: Connection from {self.peername} closing due to close_on_conn')
self.transport.close()
return

def connection_lost(self, exc):
self.logger.log(f' {self.name}: Connection to {self.peername} lost, exception={exc}')
Expand Down
3 changes: 2 additions & 1 deletion tests/system_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ def get_local_host_socket(socket_address_family='IPv4'):
elif socket_address_family == 'IPv6':
s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
host = '::1'

else:
raise Exception(f"Invalid socket_address_family: {socket_address_family}")
return s, host


Expand Down
2 changes: 2 additions & 0 deletions tests/system_tests_topology.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ def on_message(self, event):
router = 'C'
elif event.receiver == self.routers['D']['mgmt_receiver']:
router = 'D'
else:
raise Exception("Unexpected event receiver")
Copy link
Contributor

Choose a reason for hiding this comment

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

which linter warning made you remove these raise Exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"router" may be used before initialization.
Which isn't true for the code as it stands now, but I'd rather not suppress the error in case we modify the test at some point and miss updating the if statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

router may be used before initialization

From the actual context this doesn't appear possible, but yeah there is a hole in the code where router is never set. Rather than suppress the warning I added the Exception as a "defensive coding" approach. This test has a history of being somewhat flaky so I'm sure we'll be hacking at it at some point in the future. Should we change the code in some way where we add a new event receiver then the existing if statement would fail silently and router would in fact be uninitialized. The exception would prevent that foot-gun.


# ----------------------------------------------------------------
# This is a management message.
Expand Down
3 changes: 1 addition & 2 deletions tests/system_tests_topology_disposition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,8 +1150,7 @@ def quick_print_unknown_messages(self) :
count = count + 1
if first == -1 :
first = i
if i > last :
last = i
last = i

print(' first : %s sent : %.6lf' % (first, self.message_times[str(first)]))
print(' last : %s sent : %.6lf' % (last, self.message_times[str(last)]))
Expand Down
4 changes: 2 additions & 2 deletions tests/system_tests_two_routers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,9 @@ def on_message(self, event):
if event.message.properties['statusDescription'] != 'OK':
error = "Expected statusDescription to be OK but instead got %s" % \
event.message.properties['statusDescription']
elif event.message.body['adminStatus'] != self.deleted_admin_status:
else:
error = "Expected adminStatus to be %s but instead got %s" % \
(self.deleted_admin_status, event.message.properties['adminStatus'])
(self.deleted_admin_status, event.message.properties['adminStatus'])
self.bail(error)

elif event.receiver == self.mgmt_receiver_2:
Expand Down
Loading