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

🏗️(backends) unify data backends s3 #297

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

wilbrdt
Copy link
Contributor

@wilbrdt wilbrdt commented Apr 13, 2023

Purpose

Storage and Database backends had similar interfaces and usage, a new Data backend interface has been created.

Proposal

Update S3 backend to follow new Data backend interface.
Add pydantic model for a history entry. → moved to #317

  • Add S3DataBackend
  • Add tests for S3

Sorry, something went wrong.

@wilbrdt wilbrdt self-assigned this Apr 13, 2023
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 6f9a297 to 1086725 Compare April 17, 2023 14:12
@SergioSim SergioSim force-pushed the unify-data-backends-fs branch 2 times, most recently from c6e5c76 to e2fca1d Compare April 19, 2023 08:40
@SergioSim SergioSim force-pushed the unify-data-backends-fs branch 4 times, most recently from be10204 to b73010e Compare April 25, 2023 09:06
Base automatically changed from unify-data-backends-fs to unify-data-backends April 25, 2023 09:13
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 1086725 to 1938d8f Compare April 25, 2023 16:30
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 1938d8f to 3246a4d Compare May 3, 2023 17:33
@wilbrdt wilbrdt marked this pull request as ready for review May 3, 2023 17:34
@wilbrdt wilbrdt requested review from jmaupetit, Ardawo and SergioSim May 3, 2023 17:34
@wilbrdt wilbrdt added needs review and removed WIP labels May 3, 2023
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch 2 times, most recently from 48a15ec to 76389a3 Compare May 5, 2023 08:48
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
tests/backends/data/test_s3.py Outdated Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends branch 3 times, most recently from dfbe6e8 to b5c749f Compare May 30, 2023 10:07
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 76389a3 to 6826ccd Compare June 7, 2023 14:53
@wilbrdt wilbrdt removed the request for review from Ardawo June 7, 2023 14:54
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 044872b to 60930e0 Compare June 16, 2023 10:17
@wilbrdt wilbrdt force-pushed the unify-data-backends branch from b5c749f to f4d4b9b Compare June 16, 2023 14:21
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch 4 times, most recently from f9880ff to 87a8c5f Compare June 19, 2023 09:51
Copy link
Collaborator

@SergioSim SergioSim left a comment

Choose a reason for hiding this comment

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

Looks great) 👍
Have only minor questions/suggestions)
image

tests/fixtures/backends.py Show resolved Hide resolved
tests/fixtures/backends.py Outdated Show resolved Hide resolved
tests/fixtures/backends.py Outdated Show resolved Hide resolved
tests/fixtures/backends.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from caaf5b6 to 23e0b3f Compare June 20, 2023 17:51
@quitterie-lcs quitterie-lcs added this to the 4.0 milestone Jun 20, 2023
Copy link
Collaborator

@SergioSim SergioSim left a comment

Choose a reason for hiding this comment

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

Looks Cool) 👍
Have only one question about the use of put_object in the write method.
image

src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
tests/backends/data/test_s3.py Outdated Show resolved Hide resolved
tests/backends/data/test_s3.py Outdated Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from f6583dd to bcffe80 Compare June 27, 2023 10:39
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch 2 times, most recently from 44dd2c8 to 19c1a28 Compare June 30, 2023 09:09
Copy link
Collaborator

@SergioSim SergioSim left a comment

Choose a reason for hiding this comment

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

Awesome! That's great news that it works with StreamingIterator! Thanks for going the extra mile)
Have only one final question about the support of Iterators that are generators.

image

src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
src/ralph/backends/data/s3.py Outdated Show resolved Hide resolved
tests/backends/data/test_s3.py Outdated Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch 2 times, most recently from 0992d6e to 4cb336b Compare June 30, 2023 13:26
Copy link
Collaborator

@SergioSim SergioSim left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

image

src/ralph/backends/data/s3.py Show resolved Hide resolved
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from a687cd2 to 7e26be5 Compare July 3, 2023 07:39
@wilbrdt wilbrdt force-pushed the unify-data-backends branch from f429b3a to addc298 Compare July 3, 2023 10:32

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Add S3 backend under the new common `data` interface
@wilbrdt wilbrdt force-pushed the unify-data-backends-s3 branch from 7e26be5 to 3ed58a9 Compare July 3, 2023 10:33
@wilbrdt wilbrdt merged commit 8843a38 into unify-data-backends Jul 3, 2023
@wilbrdt wilbrdt deleted the unify-data-backends-s3 branch July 3, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants