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 an issue calling incorrect api #253

Merged
merged 1 commit into from
May 31, 2021

Conversation

ehossack
Copy link
Contributor

This commit fixes a typo in calling the incorrect restrictions api (v1 version)

@ehossack
Copy link
Contributor Author

I'm surprised there's no tests for the get_download_url_for_file* apis.

I'm hoping this is grounds for a 1.8.1 release as it breaks the download url api.

Additionally, some guidance (outside of this PR, updating your own docs) on how to have all the available versions of python for nox would be appreciated. For example macOS no longer supports 3.6.x python versions.

@ppolewicz
Copy link
Collaborator

I think nox used pyenv to install all the python versions it wanted - I didn't install anything by myself and a full suite runs well on my box (except 3.5?), so I think the comment which suggests that you need to install all python versions on your OS by yourself is not accurate.

It would normally be important enough for a 1.8.1 release, but I think we'll have a 1.9.0 early next week with all the sync changes and v2 support anyway.

@mlech-reef could you please confirm what the situation is with installing python versions for the full nox suite to run tests locally? (we shouldn't merge the comment in if it's inaccurate)

@ppolewicz
Copy link
Collaborator

(I had to approve the PR for the tests to run)

@ehossack
Copy link
Contributor Author

I think nox used pyenv to install all the python versions it wanted - I didn't install anything by myself and a full suite runs well on my box (except 3.5?), so I think the comment which suggests that you need to install all python versions on your OS by yourself is not accurate.

It would normally be important enough for a 1.8.1 release, but I think we'll have a 1.9.0 early next week with all the sync changes and v2 support anyway.

@mlech-reef could you please confirm what the situation is with installing python versions for the full nox suite to run tests locally? (we shouldn't merge the comment in if it's inaccurate)

Makes sense if the comment is innacurate. I have pyenv configured and running any of the nox commands failed, and I couldn't get nox to figure out how to pick up my installed python versions - hence the "which python" in there.
Also python 3.5 and 3.6 fail to install on my system (macOS 11) due to some limitations, just to keep that in mind for tests/prs. For my own project I just run all the commands in a docker container...

@ppolewicz
Copy link
Collaborator

Maybe it's a bug on macos, lets see what @mlech-reef will have to say about it

@mlech-reef
Copy link
Collaborator

mlech-reef commented May 30, 2021

I don't think nox installs python on its own. It never happens for me :) It uses versions from ['3.5', '3.6', '3.7', '3.8', '3.9'] by default. That list could be overridden via PYTHON_VERSIONS environment variable (it is documented in the developer guide). If the version is not available on the PATH, it will fail, e.g.:
image

If you have properly configured global versions in pyenv like:
image
or local, e.g.:
image

everything should work fine.

@ehossack
Copy link
Contributor Author

So the comment itself is correct. And CI provides a backup.

test/unit/v_all/test_api.py Outdated Show resolved Hide resolved
test/unit/v_all/test_api.py Outdated Show resolved Hide resolved
This commit fixes a typo in calling the incorrect
restrictions api (v1 version)
@ehossack ehossack force-pushed the fix/bucket-name-restrictions branch from 9563c96 to 8fe7a35 Compare May 31, 2021 15:24
@ehossack
Copy link
Contributor Author

Moved the tangential conversation around local setup to #255

@ppolewicz
Copy link
Collaborator

ok, my mistake, I was under the impression that nox will install environments for itself

@ppolewicz ppolewicz merged commit de26adf into Backblaze:master May 31, 2021
@ppolewicz
Copy link
Collaborator

Great contribution, added a test. I think it's not the first thing @ehossack contributed, right? Perhaps it's time for some more ambitious work we could do together?

@ehossack
Copy link
Contributor Author

Great contribution, added a test. I think it's not the first thing @ehossack contributed, right? Perhaps it's time for some more ambitious work we could do together?

For now I will have to stay with the smaller contributions to save my time!

@ehossack ehossack deleted the fix/bucket-name-restrictions branch May 31, 2021 16:08
@ehossack
Copy link
Contributor Author

P.S. When can we expect 1.9.0 if no 1.8.1? 😛

@ppolewicz
Copy link
Collaborator

It's very close now. @mpnowacki-reef do we have anything left for 1.9.0? A couple new options for sync I think?

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.

3 participants