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

Jump to bottom

Add types to rcutils_logger.py #1249

Merged
merged 14 commits into from
Aug 5, 2024
Merged

Add types to rcutils_logger.py #1249

merged 14 commits into from
Aug 5, 2024

Conversation

InvincibleRMC
Copy link
Contributor

No description provided.

@InvincibleRMC
Types to rcutils_logger.py
44603e5 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
remove unused types
7ec9181 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Add back empty dictionary
91b4771 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@fujitatomoya fujitatomoya self-requested a review March 21, 2024 19:40
@InvincibleRMC
Merge branch 'rolling' into rcutils-logger
58d6fad
@InvincibleRMC
rerun ci
69ab5da 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
update
0a68da4 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Add ClassVar
4b0aa25 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
re-run
585c872 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
re-run
bc06a7a 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
fujitatomoya
fujitatomoya approved these changes Jul 30, 2024
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm with green CI

@fujitatomoya
Copy link
Collaborator

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@InvincibleRMC
Remove explicit TypeAlias for older python versions
0689ec7 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Contributor Author

CI:

* Linux [![Build Status](https://camo.githubusercontent.com/8c169b4e9000ad061b716f61e60ddec43cb67dd841e877e30a76c46c1024d952/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e7578266275696c643d3231353835)](http://ci.ros2.org/job/ci_linux/21585/)

* Linux-aarch64 [![Build Status](https://camo.githubusercontent.com/7756bdd3d5acc9a83293429dcd85cfeadff5347020903d99af6a1b808d0a3d9e/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e75782d61617263683634266275696c643d3135383734)](http://ci.ros2.org/job/ci_linux-aarch64/15874/)

* Linux-rhel [![Build Status](https://camo.githubusercontent.com/0899416802b71cd8a97853c6c5212afb37a72aa7e933f0d37235d6955785bc47/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e75782d7268656c266275696c643d31303834)](http://ci.ros2.org/job/ci_linux-rhel/1084/)

* Windows [![Build Status](https://camo.githubusercontent.com/1e63ec047c780685355f9fc8855bcf3c2512bdaae3cc3cb0be1f6baee9f2d646/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f77696e646f7773266275696c643d3232333035)](http://ci.ros2.org/job/ci_windows/22305/)

Fixed RHEL failing

@InvincibleRMC
Fix error suppresion
7d5dd88 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Switch to ModuleNotFoundError
f3883b0 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Contributor Author

@fujitatomoya Can you re-run the CI? It should be fixed now.

@fujitatomoya
Copy link
Collaborator

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@InvincibleRMC
Use typing.OrderedDict for python 3.8 support
a8c61ab 
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Contributor Author

@fujitatomoya the Windows failure should be fixed.

fujitatomoya
fujitatomoya reviewed Aug 1, 2024
View reviewed changes
rclpy/rclpy/impl/rcutils_logger.py
@@ -25,6 +24,7 @@
from typing import Literal
from typing import NamedTuple
from typing import Optional
from typing import OrderedDict
Copy link
Collaborator

Choose a reason for hiding this comment

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

this fixes, https://ci.ros2.org/job/ci_windows/22308/testReport/junit/(root)/rclpy/___/

..\..\src\ros2\rclpy\rclpy\rclpy\logging.py:21: in <module>
    from rclpy.impl.rcutils_logger import RcutilsLogger
..\..\src\ros2\rclpy\rclpy\rclpy\impl\rcutils_logger.py:254: in <module>
    supported_filters: OrderedDict[SupportedFiltersKeys, Type[LoggingFilter]] = OrderedDict()
E   TypeError: 'type' object is not subscriptable

@fujitatomoya
Copy link
Collaborator

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@InvincibleRMC
Copy link
Contributor Author

@fujitatomoya Should Windows still be building?

@sloretz
Copy link
Contributor

sloretz commented Aug 2, 2024

Pulls: #1249
Gist: https://gist.githubusercontent.com/sloretz/9b513f6f620dde4280acfd272da86108/raw/a1ba744672f1839cfb10e716abe6627626997d69/ros2.repos
BUILD args: --packages-up-to rclpy
TEST args: --packages-select rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14361

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@sloretz sloretz merged commit 34fe579 into ros2:rolling Aug 5, 2024
2 checks passed
@MichaelOrlov
Copy link

MichaelOrlov commented Aug 9, 2024 •
edited
Loading

@sloretz @clalancette This PR caused regression in multiple tests in rosbag2 with the error message:

2024-08-06T09:14:51.2768272Z 4:     if sys.version_info >= (3, 12):
2024-08-06T09:14:51.2772576Z 4:         from typing import Unpack
2024-08-06T09:14:51.2773242Z 4:     else:
2024-08-06T09:14:51.2773636Z 4:         with suppress(ModuleNotFoundError):
2024-08-06T09:14:51.2774252Z 4: >           from typing_extensions import Unpack
2024-08-06T09:14:51.2775655Z 4: E           ImportError: cannot import name 'Unpack' from 'typing_extensions' (/usr/lib/python3/dist-packages/typing_extensions.py)
2024-08-06T09:14:51.2776690Z 4: 
2024-08-06T09:14:51.2777501Z 4: ../../install/rclpy/lib/python3.10/site-packages/rclpy/impl/rcutils_logger.py:41: ImportError

You can see a full log from one of the failing jobs

Curious if we can revert this PR?

@clalancette
Copy link
Contributor

So we aren't seeing this failing in e.g. https://ci.ros2.org/view/nightly/job/nightly_linux_release/3164/ . My best guess is that the action there is using Ubuntu 22.04, instead of Ubuntu 24.04. Since we are only targeting Ubuntu 24.04, that action should probably be updated.

@sloretz
Copy link
Contributor

sloretz commented Aug 9, 2024

My best guess is that the action there is using Ubuntu 22.04, instead of Ubuntu 24.04. Since we are only targeting Ubuntu 24.04, that action should probably be updated.

Looks like the action is indeed running 22.04

Operating System
Ubuntu
22.04.4
LTS

@sloretz sloretz mentioned this pull request Aug 9, 2024
Closed
@MichaelOrlov
Copy link

Ok. Got it.

@InvincibleRMC InvincibleRMC deleted the rcutils-logger branch September 13, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@sloretz sloretz sloretz approved these changes

@adityapande-1995 adityapande-1995 Awaiting requested review from adityapande-1995 adityapande-1995 is a code owner

Assignees

@sloretz sloretz

@fujitatomoya fujitatomoya

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@InvincibleRMC @fujitatomoya @sloretz @MichaelOrlov @clalancette