This PR fixes a regression of peach-dyndns-updater. I still don't totally understand the cause of the regression (maybe upgrade of rust?), so I've just changed to use nsupdate in a slightly different way which is working now (writing the arguments to a file, and then calling nsupdate passing the path to the file as an argument, instead of instantiating a child process and passing the arguments by writing to stdin for the child).
I've also
added the ability to configure the URL for the peach-dyndns-server (to allow for more easily testing a development server... important for fixing stuff when this is live),
added some brief documentation of the configs
fixed some clippy warnings
I've tested on the pi that this is working.
This PR fixes a regression of peach-dyndns-updater. I still don't totally understand the cause of the regression (maybe upgrade of rust?), so I've just changed to use nsupdate in a slightly different way which is working now (writing the arguments to a file, and then calling nsupdate passing the path to the file as an argument, instead of instantiating a child process and passing the arguments by writing to stdin for the child).
I've also
- added the ability to configure the URL for the peach-dyndns-server (to allow for more easily testing a development server... important for fixing stuff when this is live),
- added some brief documentation of the configs
- fixed some clippy warnings
I've tested on the pi that this is working.
I've just changed to use nsupdate in a slightly different way which is working now (writing the arguments to a file, and then calling nsupdate passing the path to the file as an argument, instead of instantiating a child process and passing the arguments by writing to stdin for the child).
Nice, that sounds like a good approach to me.
Great PR! Very happy to have this in working order :) Now we'll have two ways of testing inter-peach replication (DYN DNS & Tor hidden service).
I added a couple commits:
Using idiomatic paths for config_manager function calls
Ran cargo fmt to tidy-up a couple small formatting issues
Replaced the .unwrap() with Result<bool, PeachError> + ? for the function which checks if the dyn dns domain is new
Added a ? operator for the function call in peach-web
This unwrap() bit me when experimenting with rouille and maud (server crash when visiting the DNS settings page) - figured I'd take care of it while my memory was fresh
@notplants
> I've just changed to use nsupdate in a slightly different way which is working now (writing the arguments to a file, and then calling nsupdate passing the path to the file as an argument, instead of instantiating a child process and passing the arguments by writing to stdin for the child).
Nice, that sounds like a good approach to me.
Great PR! Very happy to have this in working order :) Now we'll have two ways of testing inter-peach replication (DYN DNS & Tor hidden service).
I added a couple commits:
- Using idiomatic paths for `config_manager` function calls
- Ran `cargo fmt` to tidy-up a couple small formatting issues
- Replaced the `.unwrap()` with `Result<bool, PeachError>` + `?` for the function which checks if the dyn dns domain is new
- Added a `?` operator for the function call in `peach-web`
- This `unwrap()` bit me when experimenting with `rouille` and `maud` (server crash when visiting the DNS settings page) - figured I'd take care of it while my memory was fresh
Ah, this turned into a nightmare. I noticed there was a conflict for Cargo.toml for your PR and then realised that the branch you had forked from to create fix_regression was not up to date with main. Ironically, the fix_regression PR would have introduced multiple regressions.
I think I have merged all the outstanding changes: PR #60, PR #61, & PR #62.
I also discovered an issue with compilation at the workspace-level. Because the miniserde_support feature flag is enabled for peach-lib in one crate and the serde_support flag is enabled in another crate, cargo fails with errors like this:
error[E0119]: conflicting implementations of trait miniserde::Serializefor typestats::MemStat``
This is annoying. Here's a small SO post about it:
@notplants
Ah, this turned into a nightmare. I noticed there was a conflict for `Cargo.toml` for your PR and then realised that the branch you had forked from to create `fix_regression` was not up to date with `main`. Ironically, the `fix_regression` PR would have introduced multiple regressions.
I think I have merged all the outstanding changes: [PR #60](https://git.coopcloud.tech/PeachCloud/peach-workspace/pulls/60), [PR #61](https://git.coopcloud.tech/PeachCloud/peach-workspace/pulls/61), & [PR #62](https://git.coopcloud.tech/PeachCloud/peach-workspace/pulls/62).
I also discovered an issue with compilation at the workspace-level. Because the `miniserde_support` feature flag is enabled for `peach-lib` in one crate and the `serde_support` flag is enabled in another crate, `cargo` fails with errors like this:
`error[E0119]: conflicting implementations of trait `miniserde::Serialize` for type `stats::MemStat``
This is annoying. Here's a small SO post about it:
https://stackoverflow.com/questions/64200237/multiple-versions-of-local-dependency-in-cargo-workspace-with-different-features
I'll figure out a solution later.
glyph
merged commit cd1fb697f7 into main2022-01-12 09:58:46 +00:00
fix imports
... I was less familiar with those things in the spring when I was working on this and glad to feel more comfortable now
don't totally understand the merge conflict issue you ran into, but sounds like a mess and glad you got it sorted
@glyph with your original fixing commits, thanks,
- good to get rid of the unwrap
- fix imports
... I was less familiar with those things in the spring when I was working on this and glad to feel more comfortable now
don't totally understand the merge conflict issue you ran into, but sounds like a mess and glad you got it sorted
It happens to me sometimes when I work on a branch without merging latest changes from main (so my branch then ends up being behind in some ways). I think in this case the particular changes you didn't have on your branch were from the PR which updated the crypto in peach-lib (when we switched to using the sha3 crate). So then there was a conflict in the Cargo.toml of peach-lib.
I figured I'd just go ahead and make the commits rather than asking you to make changes. That also makes it easier for you to get straight to work on other tasks whenever you start your day :)
@notplants
It happens to me sometimes when I work on a branch without merging latest changes from `main` (so my branch then ends up being behind in some ways). I think in this case the particular changes you didn't have on your branch were from the PR which updated the crypto in `peach-lib` (when we switched to using the `sha3` crate). So then there was a conflict in the `Cargo.toml` of `peach-lib`.
I figured I'd just go ahead and make the commits rather than asking you to make changes. That also makes it easier for you to get straight to work on other tasks whenever you start your day :)
I think I understand, that something about the Cargo.toml from the old branch + what I added, cauased a conflict, so it couldn't just straight-forward merge in the commits it was still missing.
and cool yes I have no problem at all with less polishing commits to do :)
I think I understand, that something about the Cargo.toml from the old branch + what I added, cauased a conflict, so it couldn't just straight-forward merge in the commits it was still missing.
and cool yes I have no problem at all with less polishing commits to do :)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
This PR fixes a regression of peach-dyndns-updater. I still don't totally understand the cause of the regression (maybe upgrade of rust?), so I've just changed to use nsupdate in a slightly different way which is working now (writing the arguments to a file, and then calling nsupdate passing the path to the file as an argument, instead of instantiating a child process and passing the arguments by writing to stdin for the child).
I've also
I've tested on the pi that this is working.
@notplants
Nice, that sounds like a good approach to me.
Great PR! Very happy to have this in working order :) Now we'll have two ways of testing inter-peach replication (DYN DNS & Tor hidden service).
I added a couple commits:
config_managerfunction callscargo fmtto tidy-up a couple small formatting issues.unwrap()withResult<bool, PeachError>+?for the function which checks if the dyn dns domain is new?operator for the function call inpeach-webunwrap()bit me when experimenting withrouilleandmaud(server crash when visiting the DNS settings page) - figured I'd take care of it while my memory was fresh@notplants
Ah, this turned into a nightmare. I noticed there was a conflict for
Cargo.tomlfor your PR and then realised that the branch you had forked from to createfix_regressionwas not up to date withmain. Ironically, thefix_regressionPR would have introduced multiple regressions.I think I have merged all the outstanding changes: PR #60, PR #61, & PR #62.
I also discovered an issue with compilation at the workspace-level. Because the
miniserde_supportfeature flag is enabled forpeach-libin one crate and theserde_supportflag is enabled in another crate,cargofails with errors like this:error[E0119]: conflicting implementations of traitminiserde::Serializefor typestats::MemStat``This is annoying. Here's a small SO post about it:
https://stackoverflow.com/questions/64200237/multiple-versions-of-local-dependency-in-cargo-workspace-with-different-features
I'll figure out a solution later.
Note: I did not delete the branch, just in case we have to revert the merge.
@glyph with your original fixing commits, thanks,
... I was less familiar with those things in the spring when I was working on this and glad to feel more comfortable now
don't totally understand the merge conflict issue you ran into, but sounds like a mess and glad you got it sorted
@notplants
It happens to me sometimes when I work on a branch without merging latest changes from
main(so my branch then ends up being behind in some ways). I think in this case the particular changes you didn't have on your branch were from the PR which updated the crypto inpeach-lib(when we switched to using thesha3crate). So then there was a conflict in theCargo.tomlofpeach-lib.I figured I'd just go ahead and make the commits rather than asking you to make changes. That also makes it easier for you to get straight to work on other tasks whenever you start your day :)
I think I understand, that something about the Cargo.toml from the old branch + what I added, cauased a conflict, so it couldn't just straight-forward merge in the commits it was still missing.
and cool yes I have no problem at all with less polishing commits to do :)