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

Update lockfile handler #332

Merged
merged 23 commits into from
Dec 14, 2022
Merged

Update lockfile handler #332

merged 23 commits into from
Dec 14, 2022

Conversation

RogerZhongAWS
Copy link
Contributor

Motivation

  • Changes lockfile behavior in accordance to usability needs.

Modifications

Change summary

  • now writes thing name to lockfile

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@RogerZhongAWS RogerZhongAWS changed the base branch from main to pre-release September 22, 2022 23:34
@shangabl
Copy link
Contributor

shangabl commented Nov 8, 2022

Can you add an integration test for this? Or at least remove the part of the integration tests entry-point.sh script that create the /run/lock directory and make sure it still works. We may want to add a test though where we try to run DC again and verify it fails.

@RogerZhongAWS RogerZhongAWS changed the base branch from pre-release to main November 22, 2022 21:17
integration-tests/entry-point.sh Outdated Show resolved Hide resolved
integration-tests/entry-point.sh Outdated Show resolved Hide resolved
integration-tests/entry-point.sh Show resolved Hide resolved
integration-tests/entry-point.sh Outdated Show resolved Hide resolved
./aws-iot-device-client
retVal=$?
if [ $retVal -ne 1 ]; then
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add more comments walking through this... This is actually an integration test but it doesn't say that.

Lets explicitly call this out as as a test, explain each step.

And can you add some stdout for the failure case? If our integration tests just stop running because of a lock file issue we want to know why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the test to no longer check for the return code of device client (in the case where device client keeps running and does not exit) but instead checks for the return code of pkill after we kill the dummy process first. This value will be 1 if there is no such process named 'aws-iot-device-client'

shangabl
shangabl previously approved these changes Nov 23, 2022
@RogerZhongAWS
Copy link
Contributor Author

note: add exception handler for stoi call

@RogerZhongAWS RogerZhongAWS merged commit bb7ff67 into main Dec 14, 2022
@RogerZhongAWS RogerZhongAWS deleted the fix-lockfile branch December 14, 2022 19:28
RogerZhongAWS added a commit that referenced this pull request Jan 3, 2023
* fixes bug of device client not starting when filesystem requirements are not met
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants