38 improve google login retry behaviour - #45
Draft
rob-h-w wants to merge 13 commits into
Draft
Conversation
Especially the unsafe use of NodeHandles and the test logger.
Because otherwise we're waiting for another init signal that will never come.
To make it possible to write a test client.
So Google tests can be added.
To define the basic Google World struct & execution mechanism. `World` doesn't support explicit lifetimes, so storing `Future`s in the `GoogleWorld` struct isn't an option because they'd all have to live `'static`.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
- Coverage 81.64% 73.77% -7.87%
==========================================
Files 47 54 +7
Lines 4216 4485 +269
==========================================
- Hits 3442 3309 -133
- Misses 774 1176 +402 ☔ View full report in Codecov by Sentry. |
And that the semaphore permit is released on initialisation.
And make `source.run` execute always - the run action just waits until the `source.run`'s future has released a semaphore permit. The assumption is that this means the source has started running, and other signals can be sent. Only do any of this once.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rob-h-w
force-pushed
the
38-improve-google-login-retry-behaviour
branch
from
January 9, 2025 21:17
ce4dc9e to
d3aab98
Compare
rob-h-w
force-pushed
the
38-improve-google-login-retry-behaviour
branch
from
January 10, 2025 18:16
d3aab98 to
c3b6e4c
Compare
Incomplete because it needs to be able to mock things.
So they can depend on `mockall` without making the Cloud Scraper binary need to depend on it.
rob-h-w
force-pushed
the
38-improve-google-login-retry-behaviour
branch
from
January 11, 2025 22:45
c3b6e4c to
8ab2057
Compare
The binary path for execution is wrong only in the CI build for coverage. Use resolver behaviour "2". Move the profiles from the cloud_scraper bin to the workspace. Expose the binary path when spawning a child process fails.
This hadn't been happening.
Added by the cargo template.
| use crate::domain::config::Config; | ||
| use crate::domain::mpsc_handle::{one_shot, OneshotMpscSenderHandle}; | ||
| use crate::domain::node::{LifecycleChannelHandle, Manager}; | ||
| use crate::domain::oauth2::{ApplicationSecret, ExtraParameters}; |
Check warning
Code scanning / clippy
unused imports: `ApplicationSecret` and `ExtraParameters`
| use crate::domain::config::Config; | ||
| use crate::domain::mpsc_handle::{one_shot, OneshotMpscSenderHandle}; | ||
| use crate::domain::node::{LifecycleChannelHandle, Manager}; | ||
| use crate::domain::oauth2::{ApplicationSecret, ExtraParameters}; |
Check warning
Code scanning / clippy
unused imports: `ApplicationSecret` and `ExtraParameters`
| use core::time::Duration; | ||
| #[cfg(test)] | ||
| use mockall::automock; | ||
| use std::path::Path; |
Check warning
Code scanning / clippy
unused import: `std::path::Path`
So test dependencies can be injected into the Google integration. Doing this statically with generics failed because of rust-lang/rust#100013. Refactor to used boxed vtable-dispatching types. It's much less elegant.
rob-h-w
force-pushed
the
38-improve-google-login-retry-behaviour
branch
from
January 31, 2025 22:26
adc0a0f to
2174e33
Compare
| Self: Sized + Sync + 'async_trait, | ||
| { | ||
| Box::pin( | ||
| async move { Ok(ConfigQuery::read_config(&State::path_for_name(name).await?).await?) }, |
Check warning
Code scanning / clippy
question mark operator is useless here
Comment on lines
+63
to
+73
| pub fn new( | ||
| auth_uri: String, | ||
| auth_provider_x509_cert_url: String, | ||
| client_email: Option<String>, | ||
| client_id: String, | ||
| client_secret: String, | ||
| client_x509_cert_url: Option<String>, | ||
| project_id: String, | ||
| redirect_uris: Vec<String>, | ||
| token_uri: String, | ||
| ) -> Self { |
Check warning
Code scanning / clippy
this function has too many arguments (9/7)
| pub(crate) async fn config_path() -> Result<PathBuf, io::Error> { | ||
| let root = State::path_for::<Self>().await?; | ||
| debug!("Root: {:?}", root); | ||
| Ok(PathBuf::from(root).join("config.yaml")) |
Check warning
Code scanning / clippy
useless conversion to the same type: `std::path::PathBuf`
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Because it wasn't retrying if the first attempt didn't work for any reason.