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

Rename and segment avro_client functionality #29

Merged

fatimarahman merged 16 commits into main from de-101/python-rewrite

Jul 17, 2024

Contributor

fatimarahman commented Jul 2, 2024 •

edited

Loading

To service DE-101 (Python rewrite of Firehose Avro decoding), I have made the following changes:

Split Avro decoding and encoding capabilities into separate classes (AvroDecoder and AvroEncoder)
Made retrieving JSON schemas a publicly accessible functionality
Added ability to decode batches of records to AvroDecoder (+ additional tests to make sure this works)

fatimarahman added 4 commits

June 26, 2024 11:04


          Avro interpreter initial update

ab740d7


          Tests are only slightly working...

e20e33c


          It's working

f284f9e


          update imports

f94b52e

fatimarahman requested a review from aaronfriedman6

July 2, 2024 14:16

fatimarahman self-assigned this


          Reformatted with black

69f12e5

Contributor Author

fatimarahman commented Jul 2, 2024

Are we still using flake8 as the linter here? Or should I switch it to black in the Makefile?

Contributor

aaronfriedman6 commented Jul 2, 2024

Are we still using flake8 as the linter here? Or should I switch it to black in the Makefile?

Yeah since this is used by multiple teams I don't think we should switch to black because it will require touching all the files and will make the git history a little confusing. Do you mind keeping the linter as flake8 and reverting the reformatting commit?

fatimarahman added 2 commits

July 2, 2024 10:04


          Revert "Reformatted with black"

fe948d8

This reverts commit 69f12e5.


          Autopep8 linter used

0cd7507

Contributor Author

fatimarahman commented Jul 2, 2024

Fixed it! @aaronfriedman6

aaronfriedman6 reviewed

View reviewed changes

Contributor

aaronfriedman6 left a comment

Looks great -- just a couple small changes plus one more function and I think it'll be good to go!

CHANGELOG.md Outdated Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

tests/test_avro_client.py Outdated Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Show resolved Hide resolved


          Added decode_batch and additional tests, also addressed minor complaints

98a5b63

fatimarahman requested a review from aaronfriedman6

July 8, 2024 17:23

aaronfriedman6 reviewed

View reviewed changes

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

tests/test_avro_client.py Outdated Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

fatimarahman added 3 commits

July 9, 2024 11:39


          Resolved grammar errors plus removed decode binary func

a90e019


          Changed test method name

1f7eb85


          Oops

03e11a4

Contributor

aaronfriedman6 commented Jul 10, 2024

I think you accidentally switched back to using the black formatter

fatimarahman and others added 2 commits

July 15, 2024 12:05


          Removed base64 decoding stage

04e26a7


          Merge branch 'main' into de-101/python-rewrite

8fb3cdf

aaronfriedman6 requested changes

View reviewed changes

Contributor

aaronfriedman6 left a comment

In addition to the comments, please reformat to flake8 and update the CHANGELOG -- then this should be good to go!

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

tests/test_avro_client.py Show resolved Hide resolved

fatimarahman mentioned this pull request

DE-101: Re-platform application to Python NYPL/firehose-avro-to-json-transformer#11

Merged

fatimarahman added 2 commits

July 17, 2024 08:24


          Changed wording

a9810e0


          Merge branch 'de-101/python-rewrite' of https://github.com/NYPL/pytho…

f9c5b43

…n-utils into de-101/python-rewrite

fatimarahman requested a review from aaronfriedman6

July 17, 2024 13:36

aaronfriedman6 assigned aaronfriedman6 and unassigned fatimarahman

aaronfriedman6 previously approved these changes

View reviewed changes

src/nypl_py_utils/classes/avro_client.py Outdated Show resolved Hide resolved

aaronfriedman6 assigned fatimarahman and unassigned aaronfriedman6


          Updated method descriptions

e06efef

fatimarahman dismissed aaronfriedman6’s stale review via

e06efef

July 17, 2024 14:17

fatimarahman assigned aaronfriedman6 and unassigned fatimarahman

aaronfriedman6 approved these changes

View reviewed changes

fatimarahman merged commit 41ef2df into main

2 checks passed

fatimarahman deleted the de-101/python-rewrite branch

July 17, 2024 14:24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet