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

Workaround for missing has_key implementation #100

Closed
wants to merge 1 commit into from

Conversation

fjetter
Copy link
Contributor

@fjetter fjetter commented Jan 22, 2020

The new storage API apparently doesn't offer any exists implementation.

This is already reported on Azure/azure-sdk-for-python#9507 and they suggest a rather crude workaround...

@fjetter fjetter force-pushed the azure_blob/has_key branch from b969715 to dccaac1 Compare January 22, 2020 16:27
@fjetter
Copy link
Contributor Author

fjetter commented Jan 22, 2020

cc @crepererum @byjott

@@ -10,5 +10,6 @@ export MINIO_ACCESS_KEY=minio
export MINIO_SECRET_KEY=miniostorage

mkdir -p ~/s3
~/minio version

~/minio --version
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently breaks master btw

@coveralls
Copy link

coveralls commented Jan 22, 2020

Coverage Status

Coverage remained the same at 73.206% when pulling e3e3829 on fjetter:azure_blob/has_key into df60b79 on mbr:master.

return self.blob_container_client.exists(key)
# https://github.com/Azure/azure-sdk-for-python/issues/9507
blob_client = self.blob_container_client.get_blob_client(key)
try:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mimic existing similar functions, I'd rather propose to implement it this way:

with map_azure_exceptions(key, ("BlobNotFound",)):
    blob_client.get_blob_properties()
    return True
return False

@fjetter
Copy link
Contributor Author

fjetter commented Jan 27, 2020

Looks to me the code is not actually tested on travis. Will investigate this...

@criemen
Copy link
Collaborator

criemen commented Jan 27, 2020

Yes, this is because the azure keys we received from microsoft are only available to pull requests from the mbr/simplekv repository.
This is a limitation put in place by the travis secret feature, so external parties cannot extract secret keys from repositories

@fjetter
Copy link
Contributor Author

fjetter commented Jan 27, 2020

@Cornelius-Riemenschneider
So PRs from outside will not trigger this?

@fjetter fjetter force-pushed the azure_blob/has_key branch from edea4f4 to e3e3829 Compare January 27, 2020 13:14
@criemen
Copy link
Collaborator

criemen commented Jan 28, 2020

PRs from the outside shouldn't run the azure tests, no. The exact details elude me, though I believe whenever we don't have azure secrets provided to the test runner, azure tests are (silently) skipped.
This is also the source of all the coverage dropped comments on all external PRs.

@fjetter
Copy link
Contributor Author

fjetter commented Feb 4, 2020

How to proceed with this? the new azure storage API still breaks with simplekv

@fjetter
Copy link
Contributor Author

fjetter commented Feb 13, 2020

status on this? Can anybody merge and release this, please?

@fjetter
Copy link
Contributor Author

fjetter commented Feb 14, 2020

If somebody gives me write access I can recreate the PR on this repo to verify the tests if this is a blocker.
Otherwise I'd also be fine if one of the maintainers does the same

@fmarczin
Copy link
Collaborator

fmarczin commented Apr 8, 2020

Replaced by #102

@fmarczin fmarczin closed this Apr 8, 2020
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.

6 participants