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

DOC-2544: Adding new doctest to support updated VSS article #2886

Merged
merged 114 commits into from
Aug 31, 2023
Merged

DOC-2544: Adding new doctest to support updated VSS article #2886

merged 114 commits into from
Aug 31, 2023

Conversation

dwdougherty
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does $ tox pass with this change (including linting)?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

Description of change

Adding a new doctest file to support an updated VSS article in the RediSearch repo.

ant1fact and others added 30 commits January 22, 2023 20:41
@ant1fact
Add missing Union type in method StreamCommands.xclaim() (#2553)
5e258a1 
`Union` was missing in front of `[List[StreamIdT], Tuple[StreamIdT]]` and VSCode was producing an error because of it.
After adding `Union` the type annotation is correctly identified by VSCode.
@kristjanvalur
Simplify the sync SocketBuffer, add type hints (#2543)
e39c7ba
@rbowen
trivial typo fix (#2566)
42604b6 
* trivial typo fix

* trivial typo fix
@gmbnomis
Fix unlink in cluster pipeline (#2562)
9e6a9b5 
Implement unlink() like delete() to make it work when
used in a cluster pipeline.
@Vivanov98
Fix issue 2540: Synchronise concurrent command calls to single-client…
428d609 
… mode. (#2568)

Co-authored-by: Viktor Ivanov <viktor@infogrid.io>
@kosuke-zhang
Fix: tuple function cannot be passed more than one argument (#2573)
31a1c0b
@prokazov @prokazov-redis
Use hiredis::pack_command to serialized the commands. (#2570)
ffbe879 
* Implemented pack command and pack bytes

* 1) refactored the command packer construction process
2) now hiredis.pack_bytes is the default choice. Though it's still possible to run redisrs-py (fix the flag in utils.py) or hiredis.pack_command (flag in connection.py)

* Switch to hiredis.pack_command

* Remove the rust extension module.

* 1) Introduce HIREDIS_PACK_AVAILABLE environment variable.
2) Extract serialization functionality out of Connection class.

* 1) Fix typo.
2) Add change log entry.
3) Revert the benchmark changes

* Ditch the hiredis version check for pack_command.

* Fix linter errors

* Revert version changes

* Fix linter issues

* Looks like the current redis-py version is 4.4.1

---------

Co-authored-by: Sergey Prokazov <sergey.prokazov@redis.com>
@SoulPancake
Fix issue 2567: NoneType check before raising exception (#2569)
9e00b91 
Co-authored-by: Anuragkillswitch <70265851+Anuragkillswitch@users.noreply.github.com>
@kristjanvalur
Fix issue 2349: Let async HiredisParser finish parsing after a Connec…
e7306aa 
…tion.disconnect() (#2557)

* A failing unittest

* Do not clear the redis-reader's state when we disconnect so that it can finish reading the final message

* Test that reading a message of two chunks after a disconnect() works.

* Add Changes

* fix typos
@uglide
Add TS.MGET example for OS Redis Cluster (#2507)
fcd8f98
@jmcbailey @dvora-h
Fix issue with pack_commands returning an empty byte sequence (#2416)
f517287 
Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>
@dvora-h
Version 4.5.0 (#2580)
5cb5712
@prokazov
Fix #2581 UnixDomainSocketConnection' object has no attribute '_comma…
2b470cb 
…nd_packer' (#2583)

* Fix #2581 UnixDomainSocketConnection' object has no attribute '_command_packer' .
Apparently there is no end-to-end tests for Unix sockets
 so automation didn't catch it.  I assume that setting up
domain sockets reliably  in dockerized environment is not
very trivial.
Added test for pack_command specifically.

* Figuring out why CI fails.
Locally:
" congratulations :)"

* Fix the test.
hiredis doesn't treat memoryviews differently.
@dvora-h
Version 4.5.1 (#2586)
fd7a79d
@Galtozzy
Fix for lpop and rpop return typing (#2590)
e9ad2a3 
Right now there is an annoying warning that these methods can't be awaited when using `redis.asyncio`, even tho it does work with no problems.
@uglide
Update README to make pip install copy-pastable on zsh (#2584)
6c708c2
@davemcphee
update json().arrindex() default values (#2611)
b546a9a 
* update json().arrindex() default values

* add unit test

* fix falsy checks

* more unit tests

* add asyncio tests

* fix lint line length

---------

Co-authored-by: Alex Schmitz <aschmitz@box.com>
@chayim
Speeding up the protocol parsing (#2596)
5588ae0 
* speeding up the protocol parser

* linting

* changes to ease
@barshaul @dvora-h
Fixed CredentialsProvider examples (#2587)
3edd49b 
Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>
@CrimsonGlory
ConnectionPool SSL example (#2605)
6d1061f
@ryin1
[types] update return type of smismember to list[int] (#2617)
a372ba4 
* update return type of smismember

* use Literal instead of int
@aksinha334 @davemcphee
@chayim @barshaul @dvora-h @CrimsonGlory @ryin1
Making search document subscriptable (#2615)
8bfd492 
* Fixed issue #2598 - make Document class subscriptable

* Last time added older file, fixed it

* retrigger checks

* update json().arrindex() default values (#2611)

* update json().arrindex() default values

* add unit test

* fix falsy checks

* more unit tests

* add asyncio tests

* fix lint line length

---------

Co-authored-by: Alex Schmitz <aschmitz@box.com>

* Speeding up the protocol parsing (#2596)

* speeding up the protocol parser

* linting

* changes to ease

* Fixed CredentialsProvider examples (#2587)

Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>

* ConnectionPool SSL example (#2605)

* [types] update return type of smismember to list[int] (#2617)

* update return type of smismember

* use Literal instead of int

* retrigger checks

* Added test for document subscriptable in tests/test_search.py

* Fixed linter issue

* retrigger checks

---------

Co-authored-by: Alex Schmitz <alex.schmitz@gmail.com>
Co-authored-by: Alex Schmitz <aschmitz@box.com>
Co-authored-by: Chayim <chayim@users.noreply.github.com>
Co-authored-by: Bar Shaul <88437685+barshaul@users.noreply.github.com>
Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>
Co-authored-by: CrimsonGlory <CrimsonGlory@users.noreply.github.com>
Co-authored-by: Raymond Yin <raymond@tryevergreen.com>
@thebarbershop
Remove redundant assignment. (#2620)
91ab12a
@sileht
fix: replace async_timeout by asyncio.timeout (#2602)
25e85e5 
async_timeout does not support python 3.11
aio-libs/async-timeout#295

And have two years old annoying bugs:
aio-libs/async-timeout#229
#2551

Since asyncio.timeout has been shipped in python 3.11, we should start
using it.

Partially fixes 2551
@chayim
Adding supported redis/library details (#2621)
c61eeb2
@zakaf @zach-iee @dvora-h
add queue_class to REDIS_ALLOWED_KEYS (#2577)
d63313b 
* add queue_class to REDIS_ALLOWED_KEYS

* fix lint

* fix lint

---------

Co-authored-by: zach.lee <zach.lee@sendbird.com>
Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>
@chayim
pypy-3.9 CI (#2608)
c871723
@woutdenolf @dvora-h
introduce AbstractConnection so that UnixDomainSocketConnection can c…
7d474f9 
…all super().__init__ (#2588)

Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com>
@kristjanvalur
Fix behaviour of async PythonParser to match RedisParser as for issue #…
1b2f408 
…2349 (#2582)

* Allow data to drain from PythonParser after connection close.

* Add Changes
@dvora-h
Version 4.5.2 (#2627)
318b114
@dwdougherty
Copy link
Contributor Author

Is there any way somebody can go ahead and merge this change? I'm currently playing whack-a-mole with the lint/syntax checker. It will be isolated (for now) on the emb-examples branch. I do need to get it in place so that the new VSS article can be merged on RediSearch.

@dwdougherty dwdougherty mentioned this pull request Aug 11, 2023
Merged
2 tasks
@dwdougherty
Copy link
Contributor Author

dwdougherty commented Aug 14, 2023 •
edited
Loading

@uglide Would you please take a look at this? You can see from the previous two commits that I'm being asked to change an import one way and, after I do, I'm being asked to change it back again. I can't move the RediSearch VSS article commit forward until this Python file is committed to emb-examples. Cc: @adrianoamaral .

@chayim
Copy link
Contributor

chayim commented Aug 16, 2023

This is occurring because everything is all sorts of out of date. I recommend:

  1. Sync your emb-examples branch with the latest master
  2. Sync this branch with emb-examples
  3. Locally, run your invoke linters again.

You'll get a concise list of changes. The ones raised by flake8 will need to be made manually. However, running black and isort without their various --check arguments will reformat accordingly.

After doing this all locally - everything checks out.

@dwdougherty
Copy link
Contributor Author

I followed @chayim 's instructions to the letter, including running flake8, isort, and black -l 80 locally before pushing. No joy.

@dmaier-redislabs
Copy link
Contributor

@chayim Can you please merge this PR. This lint checker thing is blocking us from publishing the new VSS tutorial. Can we make this check somehow optional (warning, but not blocking the merge).

@chayim
Copy link
Contributor

chayim commented Aug 31, 2023 •
edited
Loading

@chayim Can you please merge this PR. This lint checker thing is blocking us from publishing the new VSS tutorial. Can we make this check somehow optional (warning, but not blocking the merge).

I've fixed the PR. There were many missing pieces, as you can see from the multiple commits:

  1. The linter was doing the thing it was programmed to do - at some point someone changed the linter to require 80 character columns (could have been me, but doesn't sound like me - if so sorry!).. something that isn't elsewhere. That was incompatible with the other linter, fixed.
  2. There are various external requirements these examples now have - ones that don't exist in the library. CI now has a dedicated requirements file for this, as a result.
  3. I've updated the README to indicate how one can contribute.

IMHO the example text somewhere else now require updates indicating that to use these examples you need to install external dependencies. I'll leave that with you.

@uglide anything else here?

@chayim
Copy link
Contributor

chayim commented Aug 31, 2023

@uglide Got it. Restored. So folks here's the change.

Regarding #1 above, isort has its own character length rules... since apparently we're not leveraging what's already in use by redis-py (because branch). For now, I've updated the action - they do the same thing. Squash + merge.

@chayim chayim added the maintenance Maintenance (CI, Releases, etc) label Aug 31, 2023
@chayim chayim merged commit 0e56165 into redis:emb-examples Aug 31, 2023
@dwdougherty
Copy link
Contributor Author

Thank you @uglide and @chayim ! I'm very sorry to have caused so much pain.

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

@dmaier-redislabs dmaier-redislabs dmaier-redislabs approved these changes

@adrianoamaral adrianoamaral adrianoamaral approved these changes

Assignees
No one assigned
Labels
maintenance Maintenance (CI, Releases, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.