-
Notifications
You must be signed in to change notification settings - Fork 320
dbus: add GetUnitFileState method #350
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
base: main
Are you sure you want to change the base?
dbus: add GetUnitFileState method #350
Conversation
@vaspahomov thanks for your patch. I've left a few comments in review, and it would be good to have a test for this too. |
586b8f6
to
3ef922d
Compare
3ef922d
to
db69fcc
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.
Code looks fine but CI is failing. I think this is because you are trying to re-use a system service unit that may not exist on older OS version. It would be good to try with a dedicated unit instead.
func TestGetUnitFileState(t *testing.T) { | ||
conn := setupConn(t) | ||
defer conn.Close() | ||
service := "systemd-udevd.service" |
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.
This may not exist in the testing environment. It would be good to set up a dedicated temporary service unit instead.
Can you please resolve conflicts so @lucab can merge it? |
looking to call this method (GetUnitFileState) but it's just stale. Is there another way? |
return status, nil | ||
} | ||
|
||
// GetUnitFileStateContext returns UnitFileState |
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.
- You don't have to add
Context
to the method name -- all new methods accept context. - What is
UnitFileState
and where it is defined?
|
||
// GetUnitFileStateContext returns UnitFileState | ||
func (c *Conn) GetUnitFileStateContext(ctx context.Context, name string) (string, error) { | ||
var prop dbus.Variant |
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.
Can you just use string here?
@vaspahomov please let us know if you want to keep working on this (and I apologize for a late response). |
No description provided.