issuegen: tidyups to code #37
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
April 15, 2020 01:36
Noticed in passing I had not updated the target path correctly for the remove case previously. Should refactor so that the target path is defined in one place, but quickly fixing for this commit.
Avoid using an intermediate staged file to write new issue snippets to. Previously, I had added the `staged` file so that there would be a temporary "scratch" pad to write the new issue snippets to before writing to the final location (`generated`). This would mean that only completed writes to `generated` would be made, by copying from `staged`. However, the `staged` file itself is not giving proper control of concurrency of reads or writes to the `generated` location. Since writes to the file are completed by a system call to the OS, the OS will ensure that the `generated` file is valid when read. To ensure that the `generated` file is read at the proper time (i.e. when the `issuegen` script has completely finished) by other system services or users, systemd units should be used for this. In removing the `staged` file, the error in the `cat` command reported in https://github.com/rfairley/console-login-helper-messages/issues/35#issue-593088209 is avoided. Instead of exiting the script with error when concurrent writes are made, there will simply be two writes of the same content to the output file.
Clean up code by minimizing the number of variables and shortening the variable names. Remove unneeded comments, and add a few more clarifying comments. Separate code into clearer blocks for different operations (generating SSH key information, udev data, and the final output issue file).
rfairley
referenced
this pull request
Apr 15, 2020
For the same reason as in https://github.com/rfairley/console-login-helper-messages/pull/36 (copying the phrasing below): Avoid using an intermediate staged file to write new motd snippets to. Previously, I had added the `staged` file so that there would be a temporary "scratch" pad to write the new motd snippets to before writing to the final location (`generated`). This would mean that only completed writes to `generated` would be made, by copying from `staged`. However, the `staged` file itself is not giving proper control of concurrency of reads or writes to the `generated` location. Since writes to the file are completed by a system call to the OS, the OS will ensure that the `generated` file is valid when read. To ensure that the `generated` file is read at the proper time (i.e. when the `motdgen` script has completely finished) by other system services or users, systemd units should be used for this. Additionally, perform code tidyups similar to those done for `issuegen` in https://github.com/rfairley/console-login-helper-messages/pull/37.
|
Closing - replaced by #40 (so this branch can be deleted from the main repo). |
rfairley
pushed a commit
to rfairley/console-login-helper-messages
that referenced
this pull request
Apr 20, 2020
For the same reason as in coreos#36 (copying the phrasing below): Avoid using an intermediate staged file to write new motd snippets to. Previously, I had added the `staged` file so that there would be a temporary "scratch" pad to write the new motd snippets to before writing to the final location (`generated`). This would mean that only completed writes to `generated` would be made, by copying from `staged`. However, the `staged` file itself is not giving proper control of concurrency of reads or writes to the `generated` location. Since writes to the file are completed by a system call to the OS, the OS will ensure that the `generated` file is valid when read. To ensure that the `generated` file is read at the proper time (i.e. when the `motdgen` script has completely finished) by other system services or users, systemd units should be used for this. Additionally, perform code tidyups similar to those done for `issuegen` in coreos#37.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #36, with more general tidyups to the issuegen code. This is prep for more general work for #7 .