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

only use cache if username matches #1121

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

eladyn
Copy link
Member

@eladyn eladyn commented Sep 25, 2022

This is a follow-up to #1019 and reorders the credential cache logic.

Previously:

  • username and password are specified: use that
  • cached credentials exist: use that
  • else: start discovery

Now:

  • cached credentials exist for given username: use that
  • username and password are specified: use that
  • else: start discovery

Also, a cache initialization failure is properly reported to the user.

@eladyn eladyn requested a review from a team September 25, 2022 21:00
@eladyn
Copy link
Member Author

eladyn commented Sep 25, 2022

@aanno, if you've got the time, I'd be happy if you could test this version and see if it fixes the problem (again 😅).

Copy link
Member

@slondr slondr left a comment

Choose a reason for hiding this comment

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

I'll hold off on merging until @aanno confirms, but code looks great

@aanno
Copy link

aanno commented Oct 1, 2022

Well, I tried but currently I run into problems to compile. Error is:

Compiling spotifyd v0.3.3 (/root/spotifyd)
error[E0658]: use of unstable library feature 'bool_to_option'
   --> src/config.rs:593:57
    |
593 |         .map(|path| Cache::new(Some(&path), audio_cache.then_some(&path), size_limit))
    |                                                         ^^^^^^^^^
    |
    = note: see issue #80967 <https://github.com/rust-lang/rust/issues/80967> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `spotifyd` due to previous error
Error: error building at STEP "RUN set -ex;  mkdir /opt/owntone || true;   cd;   export DEBIAN_FRONTEND=noninteractive;   export FEATURES="--no-default-features --all-features --features pulseaudio_backend,portaudio_backend,alsa_backend,dbus_keyring,dbus_mpris";   ln -fs /usr/share/zoneinfo/Europe/Berlin /etc/localtime;   apt update;   apt -y install   git rustc cargo libasound2-dev libssl-dev pkg-config     portaudio19-dev libdbus-1-dev libpulse-dev libogg-dev;   echo not using git clone -b zeroconf_cache_fix https://github.com/eladyn/spotifyd.git;   echo not using git clone https://github.com/Spotifyd/spotifyd.git;   git clone -b cached_credentials_fix https://github.com/eladyn/spotifyd.git;   cd spotifyd;   cargo build $FEATURES --release;   cargo install $FEATURES --path . --locked;   cp -r target/release/spotifyd ~/.cargo /opt/owntone/;": error while running runtime: exit status 101

@eladyn
Copy link
Member Author

eladyn commented Oct 1, 2022

.then_some() should be stable since 1.62.0, so you'd just need to upgrade your toolchain to a more recent version, and it should work. :)

@aanno
Copy link

aanno commented Oct 2, 2022

Updated my building hack at https://github.com/aanno/linux-config/tree/master/build-with-docker to build with rustup (instead of the rustc that comes with ubuntu jammy).

Yes, I tested it and it is working as expected. Great work! Thank you very much!

@eladyn
Copy link
Member Author

eladyn commented Oct 2, 2022

Thank you!

@eladyn eladyn merged commit 00f52f9 into Spotifyd:master Oct 2, 2022
@eladyn eladyn deleted the cached_credentials_fix branch October 2, 2022 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants