Add package version #409

brunoocasali · 2022-02-14T22:38:57Z

Add a way to load the package version programmatically.
Change CONTRIBUTING.md and check-release scripts, since there is a new way to update the package version of the python SDK.

brunoocasali · 2022-02-14T22:39:36Z

setup.py

@@ -8,7 +9,7 @@
        "requests"
    ],
    name="meilisearch",
-    version="0.18.0",
+    version=VERSION,


I don't know if this works, is there a way to check this before accepting @alallema?

Seems to work, or at least it doesn't cause an error.

python setup.py check

If you want to further verify you could publish this branch to Test PyPI https://test.pypi.org/.

I would suggest changing it to:

version=__version__

I don't know either, to be honest, I've never verified that it works that way. But like @sanders41 said, if it doesn't cause an error, it probably will.

meilisearch/version.py

.github/scripts/check-release.sh

CONTRIBUTING.md

meilisearch/version.py

setup.py

sanders41 · 2022-02-15T00:25:57Z

setup.py

@@ -8,7 +9,7 @@
        "requests"
    ],
    name="meilisearch",
-    version="0.18.0",
+    version=VERSION,


Seems to work, or at least it doesn't cause an error.

python setup.py check

If you want to further verify you could publish this branch to Test PyPI https://test.pypi.org/.

I would suggest changing it to:

version=__version__

tests/client/test_version.py

sanders41 · 2022-02-15T00:33:06Z

tests/client/test_version.py

+def test_get_version():
+    assert re.match(r'^(\d+\.)?(\d+\.)?(\*|\d+)$', VERSION)
+
+def test_get_qualified_version():


This function doesn't seem necessary so if it is removed this test can also be removed.

tests/client/test_version.py

alallema · 2022-02-15T13:52:19Z

Hi @brunoocasali,
I don't have much to add thinks that @sanders41 had already made an excellent review 🎉 ❤️

sanders41 · 2022-02-15T14:13:47Z

I updated my suggestions based on the conversation about circular imports. The suggestions now should avoid that issue.

brunoocasali · 2022-02-15T18:40:17Z

@sanders41 @alallema can you look that now?

Regarding the idea of moving the qualified_version method to the headers, @sanders41 do you mean, create this method there? Or just use the value in a literal way?

sanders41

LGTM

sanders41 · 2022-02-15T18:54:17Z

Regarding the idea of moving the qualified_version method to the headers, @sanders41 do you mean, create this method there? Or just use the value in a literal way?

I think the only place this would ever get used is in the header so I am thinking, instead of a specific function, in _httprequests.py

from meilisearch.version import __version__

class HttpRequests:
    def __init__(self, config: Config) -> None:
        self.config = config
        self.headers = {
            'Authorization': f'Bearer {self.config.api_key}',
            'User-Agent': f'Meilisearch Python (v{__version__})',
        }
    ...

That said having a function like you have isn't "wrong" in any way so it doesn't hurt to have it, I just can't think on any other time it would be used. @alallema what do you think?

brunoocasali · 2022-02-15T20:14:39Z

That said having a function like you have isn't "wrong" in any way so it doesn't hurt to have it, I just can't think on any other time it would be used. @alallema what do you think?

Oh yes, I agree, the only place it will be used is inside of the headers. I created a function to do that because is easy to test it in isolation.

sanders41 · 2022-02-15T20:27:59Z

Oh yes, I agree, the only place it will be used is inside of the headers. I created a function to do that because is easy to test it in isolation.

OK, this makes sense.

Just a question...do you want to test a specific string like this? It has the potential to end up with failing tests that aren't really failing. I believe this happened on several of the clients when error messages changed. In this header case I can imagine it could have happened when the name changed from MeiliSearch to Meilisearch.

If you do want to keep the function to test it I think I would move it to _httprequests.py and make it private. That way if you decide you want to change the header in the future you can do it without it being a breaking change.

alallema · 2022-02-16T10:40:49Z

@brunoocasali, I think it doesn't bother to have a function but I totally agree with @sanders41, I think making it private would also avoid the breaking changes, and hopefully, we shouldn't have to change our name soon 😄

alallema

LGTM! 🍗

brunoocasali · 2022-02-16T12:22:58Z

Just a question...do you want to test a specific string like this?

Oh yes, I want because this format is what Meilisearch expects on the server-side. So if the format change I really want to be warned by tests regarding this specifications/telemetry-policies

CC: @sanders41 @alallema

alallema · 2022-02-16T12:25:06Z

bors merge

bors · 2022-02-16T12:28:21Z

Build succeeded:

433: Feature/Analytics r=alallema a=brunoocasali Added a pre-defined User-Agent header using `meilisearch.version` After the implementation the MeiliSearch server is outputting the expected 💯 `[2022-03-30T04:41:46Z INFO actix_web::middleware::logger] 192.168.208.3 "GET /indexes HTTP/1.1" 200 2 "-" "Meilisearch Python (v0.18.1)" 0.000619` Add Python support as requested here meilisearch/integration-guides#150 Related to #409 Co-authored-by: Bruno Casali <[email protected]>

brunoocasali added the enhancement New feature or request label Feb 14, 2022

brunoocasali requested a review from alallema February 14, 2022 22:38

brunoocasali commented Feb 14, 2022

View reviewed changes

sanders41 reviewed Feb 15, 2022

View reviewed changes

tests/client/test_version.py Outdated Show resolved Hide resolved

brunoocasali added 3 commits February 15, 2022 15:31

Create the version.py w/ current meilisearch-python version

6afd059

Use version= from meilisearch.version.__version__ in setup.py

ebd84d9

Add basic coverage for version data

d166f19

brunoocasali force-pushed the add-version branch from 3a5b41b to f4cb4eb Compare February 15, 2022 18:35

brunoocasali added 2 commits February 15, 2022 15:38

Update check-release script with the new way to update version

813f88d

Add type notation to the return of the qualified_version method

eebf948

brunoocasali force-pushed the add-version branch from f4cb4eb to eebf948 Compare February 15, 2022 18:38

brunoocasali requested a review from sanders41 February 15, 2022 18:39

sanders41 approved these changes Feb 15, 2022

View reviewed changes

alallema approved these changes Feb 16, 2022

View reviewed changes

bors bot merged commit 5385c87 into main Feb 16, 2022

bors bot deleted the add-version branch February 16, 2022 12:28

brunoocasali mentioned this pull request Mar 30, 2022

Feature/Analytics #433

Merged

Add package version #409

Add package version #409

Uh oh!

Conversation

brunoocasali commented Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoocasali Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

sanders41 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

alallema Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanders41 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sanders41 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alallema commented Feb 15, 2022

Uh oh!

sanders41 commented Feb 15, 2022

Uh oh!

brunoocasali commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanders41 left a comment

Choose a reason for hiding this comment

Uh oh!

sanders41 commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoocasali commented Feb 15, 2022

Uh oh!

sanders41 commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alallema commented Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alallema left a comment

Choose a reason for hiding this comment

Uh oh!

brunoocasali commented Feb 16, 2022

Uh oh!

alallema commented Feb 16, 2022

Uh oh!

bors bot commented Feb 16, 2022

Uh oh!

Uh oh!

brunoocasali commented Feb 14, 2022 •

edited

Loading

brunoocasali commented Feb 15, 2022 •

edited

Loading

sanders41 commented Feb 15, 2022 •

edited

Loading

sanders41 commented Feb 15, 2022 •

edited

Loading

alallema commented Feb 16, 2022 •

edited

Loading