[WIP]: Initial ObjectStore client implementation #331

domderen · 2022-08-11T12:39:30Z

This pull request contains a JetStream ObjectStore client implementation. It is an early draft that at the moment contains following functionalities:

Create Object Store,
Get Object Store,
Delete Object Store,
Put Object,
Get Object,

It was re-implemented from the golang code in nats.go repo.

This code is not complete yet! I'm sharing it in this form to gather some initial feedback if this is the right direction for this implementation. Any feedback is welcome :)

I think it would be good to implement ObjectStore calls around Bytes.IO types, to allow handling streams, same as the golang version does, but I went with simpler bytes implementation for now. I can try to extend it later on if it will look like a right direction.

When a NATS Server restarted behind an LB, reconnection would fail since the proper connect timeout exception was not being caught, leaving the client hanging sometimes. Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Fix issue when reconnecting against LB

Create dependencies.md

…ze-option Support set pending size to client connect options

Bumps [py](https://github.com/pytest-dev/py) from 1.8.0 to 1.10.0. - [Release notes](https://github.com/pytest-dev/py/releases) - [Changelog](https://github.com/pytest-dev/py/blob/master/CHANGELOG.rst) - [Commits](pytest-dev/py@1.8.0...1.10.0) Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Removed loop from options, deprecated from nats.connect().

Update __main__.py

domderen · 2022-08-24T13:31:47Z

Hey @wallyqs it would be great to hear your thoughts on this PR.

wallyqs · 2022-08-24T17:25:19Z

Thanks @domderen for the contribution and the rebase. Sorry for the delay, I will start looking today.

tothandras

I've started to check the cross language compatibility of the implementations, first with Go and I plan to do the same with JavaScript.
(I have a use case where I write objects in Python and read them from JavaScript.)

tothandras · 2022-09-21T18:43:43Z

nats/js/object_store.py

+
+        data = None
+        if msg.data:
+            data = base64.b64decode(msg.data)


Suggested change

data = base64.b64decode(msg.data)

data = msg.data.decode()

nats/js/object_store.py

nats/js/api.py

tothandras · 2022-09-21T18:59:49Z

nats/js/object_store.py

+            )
+            raise e
+
+        info.mtime = datetime.utcnow().timestamp()


Suggested change

info.mtime = datetime.utcnow().timestamp()

info.mtime = datetime.now(timezone.utc).isoformat()

tothandras · 2022-09-21T19:00:08Z

nats/js/object_store.py

+                nuid=id.decode(),
+                size=0,
+                chunks=0,
+                mtime=0


from datetime import datetime, timezone # ...

Suggested change

mtime=0

mtime=datetime.now(timezone.utc).isoformat(),

nats/js/object_store.py

tothandras · 2022-09-21T19:48:44Z

nats/js/object_store.py

+
+                # Make sure the digest matches.
+                sha = h.digest()
+                digest_str = info.digest.replace(OBJ_DIGEST_TYPE, "")


Suggested change

digest_str = info.digest.replace(OBJ_DIGEST_TYPE, "")

digest_str = info.digest.replace(OBJ_DIGEST_TYPE, "").replace(OBJ_DIGEST_TYPE.upper(), "")

Hey, could you elaborate, why somthing like this?

I've seen both "digest": "sha-256=xxx" and "digest": "SHA-256=xxx". Probably we should change this line instead: https://github.com/tothandras/nats.py/blob/object_store/nats/js/object_store.py#L50. The Go client uses SHA: https://github.com/nats-io/nats.go/blob/f4102422df7df83b2032318fd14a248c45f113e1/object.go#L232 and the JS/Deno client uses sha: https://github.com/nats-io/nats.deno/blob/6bdf6ff33f740e3c2c30251a79d291b1cb5288ff/tests/objectstore_test.ts#L87

nats/js/object_store.py

+        obj = self.__sanitize_name(name)
+
+        if not key_valid(obj):
+            raise InvalidObjectNameError
+
+        meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)


tothandras · 2022-09-21T20:03:04Z

nats/js/object_store.py

+        if not key_valid(obj):
+            raise InvalidObjectNameError
+
+        meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)


See: https://github.com/nats-io/nats.go/blob/f4102422df7df83b2032318fd14a248c45f113e1/object.go#L442

Suggested change

meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)

meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=base64.urlsafe_b64encode(bytes(obj, "utf-8")).decode().rstrip("="))

tothandras · 2022-09-21T20:03:52Z

nats/js/object_store.py

+        chunk_subj = OBJ_CHUNKS_PRE_TEMPLATE.format(
+            bucket=self._name, obj=id.decode()
+        )
+        meta_subj = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)


Suggested change

meta_subj = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)

meta_subj = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=base64.urlsafe_b64encode(bytes(obj, "utf-8")).decode().rstrip("="))

domderen · 2022-09-22T08:18:22Z

Hey @tothandras thanks for your comments. I'm on holidays at the moment, but will look into it once I'm back in ~2 weeks.

tothandras · 2022-09-22T10:21:46Z

@domderen enjoy your holiday!

domderen · 2022-10-05T17:44:24Z

Hey @tothandras I updated this PR with your comments. I think I included them all, but would you mind taking a look?

tothandras · 2022-10-07T16:02:02Z

@domderen If you could rebase on master I could try it out. I'm probably less suited for reviewing the code itself. I lack the deep knowledge of Python. :)

domderen · 2022-10-08T09:26:38Z

Hey I think the rebase should be done, though I'm not sure if I didn't mess something up... Looks like a lot of changes now...

wallyqs · 2022-10-08T11:29:34Z

@domderen I think you can try something like the following below to squash into a single commit and avoid the merge conflicts using your current branch:

# Checkout the object store branch
git checkout object_store

# Take a diff against latest main, we'll apply this diff as a patch.
git diff main > obj.diff

# Rename current local object_store branch.
git branch -m object-store-backup

# Redo object_store branch
git checkout main
git branch object_store
git checkout object_store

# Apply the diff as a patch on top of object_store branch and commit.
git apply -3 obj.diff

# Fix formatting
make format

git commit -s -m "Add Object Store implementation"

# Force push the object_store branch that is now squashed and rebased.
git push main object_store -f

I gave it a try at squashing into a single commit via a diff the result can be found here: https://github.com/nats-io/nats.py/compare/obj
When running the tests I'm running into a binascii.Error: Incorrect padding error though:
https://app.travis-ci.com/github/nats-io/nats.py/jobs/585091380#L508

tothandras · 2022-10-08T14:00:21Z

I just run into an issue with __sanitize_name, we should definitely write some integration tests to make sure all language implementations are compatible with each other. I hope to have time for it in the next 2-3 weeks.

Signed-off-by: Dominik Deren <dominik.deren@live.com>

domderen · 2022-10-09T10:06:55Z

Hey @wallyqs thanks for the help! I followed your steps and force pushed just those changes that looked like my original implementation. It should make it easier to review. Let's see if the build passes now, if not I'll take a look at tests.

domderen · 2022-10-09T10:08:02Z

@tothandras I think I tried to take this implementation of __sanitize_name from the golang implementation. Are they different now?

domderen · 2022-10-09T10:12:15Z

Hmm there are two tests failing, but those are two different tests in two different builds, both don't seem to be related to my changes... Should I expect some flakiness in those tests?

tothandras · 2022-10-10T09:58:41Z

@tothandras I think I tried to take this implementation of __sanitize_name from the golang implementation. Are they different now?

Sorry, I was wrong. It seems I didn't apply all the patches on my fork.

…to object_store

danielfernandez-igz · 2023-03-06T15:45:11Z

Hi everyone!

Are there any plans to resume this PR in the short term? I noticed it was reviewed six months ago, but has been stale for some time now.

Also, looks like the Travis build failed due to a flaky test but it's been excluded from pytest runs, so it shouldn't be a concern anymore (see #424).

Thank you all!

domderen · 2023-03-07T14:43:05Z

Hey, I was updating this PR occasionally when I needed to change something in the Object Store implementation for my own needs. I'm happy to keep going on this PR, but I'm not sure if owners see it as valuable, or if they would prefer a different direction.

It is also still missing some of the APIs that Object Store has in golang version, and I'm not sure what would be the minimal required scope for it to get merged.

wallyqs and others added 13 commits November 11, 2020 12:26

Fix issue reconnecting against LBs

cad4e7c

When a NATS Server restarted behind an LB, reconnection would fail since the proper connect timeout exception was not being caught, leaving the client hanging sometimes. Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Bump version

7b068da

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Add test for condition of bad server behind LB

70aa87b

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Merge pull request nats-io#189 from nats-io/reconnect-with-lbs

9f69047

Fix issue when reconnecting against LB

Create dependencies.md

7217f3c

Merge pull request nats-io#191 from nats-io/add_dependencies_doc

045f7bf

Create dependencies.md

support set pending size to client connect options

4c48f90

Merge pull request nats-io#196 from blackstorm/lee/connect-pending-si…

50034c9

…ze-option Support set pending size to client connect options

yapf: format fixes

83b5b6a

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

Merge branch 'dependabot/pip/py-1.10.0'

f011db7

Update __main__.py

3f35065

Removed loop from options, deprecated from nats.connect().

Merge pull request nats-io#294 from tdelora/patch-1

03070fc

Update __main__.py

tothandras reviewed Sep 21, 2022

View reviewed changes

nats/js/object_store.py Show resolved Hide resolved

tothandras reviewed Sep 21, 2022

View reviewed changes

nats/js/object_store.py Outdated

Comment on lines 143 to 148

obj = self.__sanitize_name(name)

if not key_valid(obj):

raise InvalidObjectNameError

meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)

This comment was marked as outdated.

Sign in to view

tothandras reviewed Sep 21, 2022

View reviewed changes

tothandras mentioned this pull request Sep 21, 2022

[FIX] cross language compatible meta subject nats-io/nats.deno#372

Merged

Add Object Store implementation

eef93c5

Signed-off-by: Dominik Deren <dominik.deren@live.com>

domderen force-pushed the object_store branch from e3d17ad to eef93c5 Compare October 9, 2022 10:05

domderen added 9 commits December 7, 2022 14:34

Merge remote-tracking branch 'origin/master' into object_store

7816cf6

Removing not wanted change.

4bfd334

Updating lock file.

c6c07ff

Fixing object store item name parser.

3a2c798

Merge branch 'main' into object_store

d308a51

Fixing code formatting.

e84d195

Merge branch 'object_store' of https://github.com/domderen/nats.py in…

2bd99de

…to object_store

Removing test that was added during incorrect merge.

166bc0a

Fixing CI issue.

cb4bf45

wallyqs mentioned this pull request Jun 1, 2023

feat: add object store feature #452

Merged

wallyqs merged commit 1daa0ca into nats-io:main Jun 1, 2023

domderen deleted the object_store branch March 6, 2024 08:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]: Initial ObjectStore client implementation #331

[WIP]: Initial ObjectStore client implementation #331

domderen commented Aug 11, 2022

domderen commented Aug 24, 2022

wallyqs commented Aug 24, 2022

tothandras left a comment

tothandras Sep 21, 2022 •

edited

tothandras Sep 21, 2022

tothandras Sep 21, 2022

tothandras Sep 21, 2022

domderen Oct 5, 2022

tothandras Oct 7, 2022 •

edited

This comment was marked as outdated.

tothandras Sep 21, 2022 •

edited

tothandras Sep 21, 2022 •

edited

domderen commented Sep 22, 2022

tothandras commented Sep 22, 2022

domderen commented Oct 5, 2022

tothandras commented Oct 7, 2022

domderen commented Oct 8, 2022

wallyqs commented Oct 8, 2022

tothandras commented Oct 8, 2022

domderen commented Oct 9, 2022

domderen commented Oct 9, 2022

domderen commented Oct 9, 2022

tothandras commented Oct 10, 2022

danielfernandez-igz commented Mar 6, 2023

domderen commented Mar 7, 2023

	info.mtime = datetime.utcnow().timestamp()
	info.mtime = datetime.now(timezone.utc).isoformat()

	digest_str = info.digest.replace(OBJ_DIGEST_TYPE, "")
	digest_str = info.digest.replace(OBJ_DIGEST_TYPE, "").replace(OBJ_DIGEST_TYPE.upper(), "")

	meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)
	meta = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=base64.urlsafe_b64encode(bytes(obj, "utf-8")).decode().rstrip("="))

	meta_subj = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=obj)
	meta_subj = OBJ_META_PRE_TEMPLATE.format(bucket=self._name, obj=base64.urlsafe_b64encode(bytes(obj, "utf-8")).decode().rstrip("="))

[WIP]: Initial ObjectStore client implementation #331

[WIP]: Initial ObjectStore client implementation #331

Conversation

domderen commented Aug 11, 2022

domderen commented Aug 24, 2022

wallyqs commented Aug 24, 2022

tothandras left a comment

Choose a reason for hiding this comment

tothandras Sep 21, 2022 • edited

Choose a reason for hiding this comment

tothandras Sep 21, 2022

Choose a reason for hiding this comment

tothandras Sep 21, 2022

Choose a reason for hiding this comment

tothandras Sep 21, 2022

Choose a reason for hiding this comment

domderen Oct 5, 2022

Choose a reason for hiding this comment

tothandras Oct 7, 2022 • edited

Choose a reason for hiding this comment

This comment was marked as outdated.

tothandras Sep 21, 2022 • edited

Choose a reason for hiding this comment

tothandras Sep 21, 2022 • edited

Choose a reason for hiding this comment

domderen commented Sep 22, 2022

tothandras commented Sep 22, 2022

domderen commented Oct 5, 2022

tothandras commented Oct 7, 2022

domderen commented Oct 8, 2022

wallyqs commented Oct 8, 2022

tothandras commented Oct 8, 2022

domderen commented Oct 9, 2022

domderen commented Oct 9, 2022

domderen commented Oct 9, 2022

tothandras commented Oct 10, 2022

danielfernandez-igz commented Mar 6, 2023

domderen commented Mar 7, 2023

tothandras Sep 21, 2022 •

edited

tothandras Oct 7, 2022 •

edited

tothandras Sep 21, 2022 •

edited

tothandras Sep 21, 2022 •

edited