-
Notifications
You must be signed in to change notification settings - Fork 28
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
Support for GDAL related assets #218
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,8 +387,8 @@ def download_asset(self, asset, load_asset=False): | |
tr("Error in downloading file, {}").format(str(e)) | ||
) | ||
|
||
def sign_asset_href(self, asset_href): | ||
""" Signs the SAS based asset href. | ||
def sign_asset_href(self, asset_href: str): | ||
""" Signs the asset href if signing is required. | ||
|
||
:param asset_href: Asset resource href | ||
:type asset_href: str | ||
|
@@ -397,13 +397,18 @@ def sign_asset_href(self, asset_href): | |
:rtype str | ||
""" | ||
|
||
# If the plugin current connection has a sas subscription key | ||
# use it instead of the environment one. | ||
sas_key = os.getenv(SAS_SUBSCRIPTION_VARIABLE) | ||
if not asset_href: | ||
return asset_href | ||
|
||
connection = settings_manager.get_current_connection() | ||
|
||
if connection and \ | ||
connection.capability == ApiCapability.SUPPORT_SAS_TOKEN: | ||
|
||
# If the plugin current connection has a sas subscription key | ||
# use it instead of the environment one. | ||
sas_key = os.getenv(SAS_SUBSCRIPTION_VARIABLE) | ||
|
||
sas_key = connection.sas_subscription_key \ | ||
if connection.sas_subscription_key else sas_key | ||
|
||
|
@@ -412,6 +417,18 @@ def sign_asset_href(self, asset_href): | |
signed_href = pc.sign(asset_href) | ||
return signed_href | ||
|
||
if asset_href.startswith("s3://"): | ||
signed_href = gdal.GetSignedURL(f"/vsis3/{asset_href[5:]}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't find documentation for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. gdal.GetSignedURL tranforms a GDAL virtual file system path to one https signed url. Depending on the virtual file system type, GDAL reads its specific authentication settings from environment variables. This is an example: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "A signed URL is a URL that provides limited permission and time to make a request. Signed URLs contain authentication information in their query string, allowing users without credentials to perform specific actions on a resource" from https://cloud.google.com/storage/docs/access-control/signed-urls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, GDAL reads several environment variables (AWS_NO_SIGN_REQUEST, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN...) to create the signed url. It is very useful custom way to auhenticate, apart from using the UI of authentication that your plugin provides. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Samweli you can use the Collection Digital Earth Africa - s2l2a in the predefined Catalog. Before, You have to define this environment variable AWS_NO_SIGN_REQUEST=YES. Using gdal 3.5.1 it works. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So these new changes will not work without user having to define the environment variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. For other use cases, credentials will be configured defining AWS_PROFILE or AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY. It depends on how the STAC Catalog is deployed in the cloud-provider. |
||
return signed_href | ||
|
||
if asset_href.startswith("gs://"): | ||
signed_href = gdal.GetSignedURL(f"/vsigs/{asset_href[5:]}") | ||
return signed_href | ||
|
||
if asset_href.startswith("/vsi"): | ||
signed_href = gdal.GetSignedURL(asset_href) | ||
return signed_href | ||
|
||
return asset_href | ||
|
||
def download_progress(self, value): | ||
|
@@ -470,8 +487,7 @@ def load_asset(self, asset): | |
point_cloud_types = ','.join([ | ||
AssetLayerType.COPC.value, | ||
]) | ||
current_asset_href = asset.href | ||
asset.href = self.sign_asset_href(asset.href) | ||
asset_href = self.sign_asset_href(asset.href) | ||
|
||
if asset_type in raster_types: | ||
layer_type = QgsMapLayer.RasterLayer | ||
|
@@ -485,20 +501,19 @@ def load_asset(self, asset): | |
) and \ | ||
asset_type != AssetLayerType.GEOTIFF.value: | ||
asset_href = f"{self.vis_url_string}" \ | ||
f"{asset.href}" | ||
f"{asset_href}" | ||
elif asset_type in ''.join([ | ||
AssetLayerType.NETCDF.value]): | ||
# For NETCDF assets type we need to download the intended asset first, | ||
# then we read from the downloaded file and use all the available NETCDF | ||
# variables on the file to load the layer. | ||
|
||
asset.downloaded = os.path.exists(asset.href) | ||
asset.downloaded = os.path.exists(asset_href) | ||
|
||
asset_href = asset.href | ||
if asset.downloaded: | ||
try: | ||
gdal.UseExceptions() | ||
open_file = gdal.Open(asset.href) | ||
open_file = gdal.Open(asset_href) | ||
if open_file is not None: | ||
file_metadata = open_file.GetMetadata( | ||
GDAL_SUBDATASETS_KEY | ||
|
@@ -510,17 +525,14 @@ def load_asset(self, asset): | |
|
||
asset_href = file_uris | ||
except RuntimeError as err: | ||
asset_href = asset.href | ||
log( | ||
tr("Runtime error when adding a NETCDF asset," | ||
" {}").format(str(err)) | ||
) | ||
else: | ||
asset.href = current_asset_href | ||
self.download_asset(asset, True) | ||
return | ||
else: | ||
asset_href = f"{asset.href}" | ||
|
||
asset_name = asset.name or asset.title | ||
self.update_inputs(False) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahuarte47 can you put the code for with the new changes into its own function the
sign_asset_href
method is for signing planetary computer SAS based items hrefs with a SAS token.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is not the generation of signed urls the purpose of
sign_asset_href
? IMHO this function can manage all cases without modifying the current behavior or the existence of new future sign methods. Do you agree? Just chaning the description of the method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make another function for the new changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Samweli can't really understand. The plugin and the sign method are not named as microsoft or planetarycomputer dependent => because the plugin is a Common, have to be and remain general.
sign_asset_href
is a enough general name that can refer to the signing action that is common to any cloud resource not only that from microsoft services.In resume, what is the rationale to move a sign feature for only GC or AWS to a different method respect signing of microsoft cloud?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahuarte47 @luipir I prefer to have another function for the new changes, at some point I ll also rename the current
sign_asset_href
to a more specific PC related name.