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

Fix graphouse.py #129

Merged
merged 8 commits into from
Aug 14, 2019
Merged

Fix graphouse.py #129

merged 8 commits into from
Aug 14, 2019

Conversation

Felixoid
Copy link
Member

@Felixoid Felixoid commented Jul 19, 2019

Since py3 strings are iterable as well, that's why #98 and Altinity/clickhouse-graphouse-integration#14 have been in place.

pep8 was applied to the code as well

@AndreevDm AndreevDm requested review from firov and alkedr and removed request for vvv1559 and AndreevDm July 24, 2019 16:55
@Felixoid Felixoid changed the title Fix graphouse py WIP: Fix graphouse py Jul 24, 2019
alkedr
alkedr previously approved these changes Aug 2, 2019
@Felixoid
Copy link
Member Author

Felixoid commented Aug 2, 2019

It doesn't work now neither in 1.0 nor in 1.1. I'm implementing the proper find_multi for finder now

@Felixoid Felixoid requested a review from alkedr August 4, 2019 17:23
@Felixoid Felixoid force-pushed the fix_graphouse_py branch 2 times, most recently from 01d645c to b0cc15f Compare August 4, 2019 18:25
@Felixoid Felixoid dismissed alkedr’s stale review August 4, 2019 18:27

That code didn't work

@Felixoid Felixoid changed the title WIP: Fix graphouse py Fix graphouse py Aug 4, 2019
@Felixoid Felixoid force-pushed the fix_graphouse_py branch 2 times, most recently from 177047c to 245c2f0 Compare August 4, 2019 19:19
@Felixoid Felixoid changed the title Fix graphouse py Fix graphouse.py Aug 5, 2019
@Felixoid Felixoid changed the title Fix graphouse.py WIP: Fix graphouse.py Aug 5, 2019
@Felixoid
Copy link
Member Author

Felixoid commented Aug 5, 2019

Всё ещё есть странные ошибки

@Felixoid Felixoid force-pushed the fix_graphouse_py branch 3 times, most recently from 8a8a554 to de1b4e9 Compare August 9, 2019 14:28
@Felixoid Felixoid changed the title WIP: Fix graphouse.py Fix graphouse.py Aug 9, 2019
@Felixoid Felixoid force-pushed the fix_graphouse_py branch 2 times, most recently from 2306a79 to 5ffa934 Compare August 9, 2019 22:55
@Felixoid
Copy link
Member Author

Felixoid commented Aug 9, 2019

Коллеги, тут готово к ревью.

сс. @alkedr @firov

Move body size calculation to the MultiFetcher class
Split requests to multiple for old graphite-web as well
Remove old unused logic from GraphouseReader.__init__
Create dummy classes Store and BaseFinder
@alkedr alkedr merged commit 0c2a1fa into master Aug 14, 2019
@blinkov blinkov unassigned alkedr and firov Sep 23, 2019
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.

None yet

3 participants