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.

Sign up for GitHub

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

Jump to bottom

SNOW-715510: Token cache for libsnowflakeclient #773

Merged
merged 25 commits into from
Feb 5, 2025
Merged

SNOW-715510: Token cache for libsnowflakeclient #773

merged 25 commits into from
Feb 5, 2025

Conversation

sfc-gh-jszczerbinski
Copy link
Collaborator

No description provided.

@sfc-gh-jszczerbinski sfc-gh-jszczerbinski requested a review from a team as a code owner November 7, 2024 19:00
sfc-gh-mkubik
sfc-gh-mkubik reviewed Nov 14, 2024
View reviewed changes
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageImpl.hpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
sfc-gh-astachowski
sfc-gh-astachowski reviewed Nov 15, 2024
View reviewed changes
cpp/lib/CredentialCache.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageWin.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageWin.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
tests/mock/test_mock_mfa_token_caching.c Show resolved Hide resolved
sfc-gh-dprzybysz
sfc-gh-dprzybysz reviewed Nov 18, 2024
View reviewed changes
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski force-pushed the SNOW-715510-mfa-token-cache-for-c-api-3 branch 3 times, most recently from bad2d5b to 2489386 Compare January 13, 2025 14:52
sfc-gh-dprzybysz
sfc-gh-dprzybysz reviewed Jan 14, 2025
View reviewed changes
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CacheFile.cpp Show resolved Hide resolved
cpp/lib/CacheFile.cpp Outdated Show resolved Hide resolved
cpp/lib/CredentialCache.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
cpp/platform/SecureStorageImpl.hpp Outdated Show resolved Hide resolved
include/snowflake/client.h Outdated Show resolved Hide resolved
tests/test_manual_connect.c Show resolved Hide resolved
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski force-pushed the SNOW-715510-mfa-token-cache-for-c-api-3 branch from b7fb71e to b6502b3 Compare January 28, 2025 12:47
sfc-gh-mhofman
sfc-gh-mhofman requested changes Jan 28, 2025
View reviewed changes
cpp/platform/FileLock.cpp Outdated Show resolved Hide resolved
include/snowflake/CredentialCache.hpp Outdated Show resolved Hide resolved
@codecov Codecov
Copy link

codecov bot commented Jan 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.23237% with 38 lines in your changes missing coverage. Please review.

Project coverage is 80.61%. Comparing base (ab172ef) to head (26b548f).

Files with missing lines Patch % Lines
cpp/lib/CacheFile.cpp 84.46% 16 Missing ⚠️
cpp/platform/FileLock.cpp 72.41% 8 Missing ⚠️
cpp/platform/SecureStorageLinux.cpp 88.88% 6 Missing ⚠️
cpp/util/Sha256.cpp 75.00% 4 Missing ⚠️
lib/client.c 25.00% 3 Missing ⚠️
lib/connection.c 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #773      +/-   ##
==========================================
+ Coverage   80.43%   80.61%   +0.18%     
==========================================
  Files         104      111       +7     
  Lines        9222     9463     +241     
==========================================
+ Hits         7418     7629     +211     
- Misses       1804     1834      +30

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sfc-gh-jszczerbinski sfc-gh-jszczerbinski force-pushed the SNOW-715510-mfa-token-cache-for-c-api-3 branch from e85806b to 54fc730 Compare January 30, 2025 16:46
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski force-pushed the SNOW-715510-mfa-token-cache-for-c-api-3 branch from 7b3886d to a37fe6b Compare January 31, 2025 11:28
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski changed the title SNOW-715510: MFA Token cache for libsnowflakeclient SNOW-715510: Token cache for libsnowflakeclient Feb 3, 2025
sfc-gh-mmishchenko
sfc-gh-mmishchenko reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@sfc-gh-mmishchenko sfc-gh-mmishchenko left a comment

Choose a reason for hiding this comment

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

Looks headers aren't ordered consistently across the changes

@sfc-gh-jszczerbinski sfc-gh-jszczerbinski requested a review from a team as a code owner February 3, 2025 11:24
sfc-gh-dprzybysz
sfc-gh-dprzybysz reviewed Feb 3, 2025
View reviewed changes
Copy link
Collaborator

@sfc-gh-dprzybysz sfc-gh-dprzybysz left a comment

Choose a reason for hiding this comment

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

I endded review before cpp/util/Sha256.cpp, tomorrow I will continue

cpp/platform/FileLock.hpp Show resolved Hide resolved
cpp/platform/SecureStorageApple.cpp Outdated Show resolved Hide resolved
sfc-gh-mhofman
sfc-gh-mhofman requested changes Feb 3, 2025
View reviewed changes
cpp/lib/CacheFile.cpp Show resolved Hide resolved
cpp/lib/CacheFile.cpp Show resolved Hide resolved
cpp/lib/CacheFile.cpp Show resolved Hide resolved
cpp/platform/SecureStorage.cpp Show resolved Hide resolved
cpp/platform/SecureStorage.cpp Show resolved Hide resolved
cpp/platform/SecureStorageLinux.cpp Show resolved Hide resolved
include/snowflake/SecureStorage.hpp Show resolved Hide resolved
include/snowflake/secure_storage.h Outdated Show resolved Hide resolved
lib/client.c Outdated Show resolved Hide resolved
lib/client.c Show resolved Hide resolved
sfc-gh-dprzybysz
sfc-gh-dprzybysz reviewed Feb 4, 2025
View reviewed changes
include/snowflake/SecureStorage.hpp Show resolved Hide resolved
include/snowflake/SecureStorage.hpp Show resolved Hide resolved
include/snowflake/SecureStorage.hpp Outdated Show resolved Hide resolved
include/snowflake/SecureStorage.hpp Outdated Show resolved Hide resolved
include/snowflake/secure_storage.h Outdated Show resolved Hide resolved
include/snowflake/secure_storage.h Show resolved Hide resolved
include/snowflake/SecureStorage.hpp Show resolved Hide resolved
tests/mock/test_mock_mfa_token_caching.c Outdated Show resolved Hide resolved
tests/test_unit_secure_storage.cpp Show resolved Hide resolved
tests/test_unit_secure_storage.cpp Show resolved Hide resolved
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski enabled auto-merge (rebase) February 5, 2025 08:23
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski enabled auto-merge (squash) February 5, 2025 08:23
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski merged commit 954c938 into master Feb 5, 2025
43 of 46 checks passed
@sfc-gh-jszczerbinski sfc-gh-jszczerbinski deleted the SNOW-715510-mfa-token-cache-for-c-api-3 branch February 5, 2025 09:01
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-astachowski sfc-gh-astachowski sfc-gh-astachowski left review comments

@sfc-gh-dprzybysz sfc-gh-dprzybysz sfc-gh-dprzybysz approved these changes

@sfc-gh-mhofman sfc-gh-mhofman sfc-gh-mhofman approved these changes

@sfc-gh-ext-simba-hx sfc-gh-ext-simba-hx Awaiting requested review from sfc-gh-ext-simba-hx

@sfc-gh-mkubik sfc-gh-mkubik Awaiting requested review from sfc-gh-mkubik

@sfc-gh-mmishchenko sfc-gh-mmishchenko Awaiting requested review from sfc-gh-mmishchenko

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sfc-gh-jszczerbinski @sfc-gh-dprzybysz @sfc-gh-mhofman @sfc-gh-astachowski @sfc-gh-mkubik @sfc-gh-mmishchenko