-
Notifications
You must be signed in to change notification settings - Fork 0
1 adapt gee authenticator node to support google credentials as input and provide new gee port object #3
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adapts the Google Earth Engine (GEE) authenticator node to support Google credentials as input and introduces a new Google Earth Engine port object for connection sharing between nodes.
Key changes:
- Introduces a new connection port type for Google Earth Engine that enables credential sharing across nodes
- Refactors the GEE Authenticate node to accept Google credentials and output a connection port object
- Updates two nodes (GEE Image Search and GEE Image) to use the new connection port instead of individual authentication
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
knime_extension/src/util/knime_utils.py | Adds comprehensive utility functions for column type checking and validation |
knime_extension/src/util/common.py | Introduces GoogleEarthEngineConnectionObject and related port type infrastructure |
knime_extension/src/nodes/io.py | Updates GEE Image Search and GEE Image nodes to use the new connection port |
knime_extension/src/nodes/authorization.py | Refactors GEE Authenticate node to Google Earth Engine Connector with credential input |
knime_extension/pixi.toml | Adds new pixi configuration file for environment management |
knime_extension/google_earth_engine_env.yml | Updates environment configuration with newer dependency versions |
config.yml | Updates project identifier in configuration |
Comments suppressed due to low confidence (2)
knime_extension/pixi.toml:12
- The version constraint '0.35.3.' may not be valid for geemap. Based on my knowledge, geemap versions follow semantic versioning like '0.35.3', and the wildcard pattern should be '0.35.' for patch-level flexibility.
geemap = "0.35.3.*"
knime_extension/pixi.toml:13
- The version constraint '1.1.1.' may not be valid for geopandas. Based on my knowledge, geopandas versions follow semantic versioning, and the wildcard pattern should be '1.1.' for patch-level flexibility.
geopandas = "1.1.1.*"
@return: True if Column is long |
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.
The comment incorrectly describes 'long' as including int32. The function checks for int32 and int64, but typically 'long' refers to 64-bit integers. Consider clarifying this as 'integer types' or correcting the description.
Checks if column is long (e.g. int32 or int64). | |
@return: True if Column is long | |
Checks if column is an integer type (int32 or int64). | |
@return: True if Column is int32 or int64 |
Copilot uses AI. Check for mistakes.
GeoPointCell, GeoLineCell, GeoPolygonCell, GeoMultiPointCell, GeoMultiLineCell, GeoMultiPolygonCell, ... | ||
@return: True if Column Type is GeoValue compatible |
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.
The documentation mentions GeoValue interface and geospatial types, but the function name is 'is_date' and it checks for LocalDateValueFactory. This appears to be incorrect documentation copied from another function.
Checks if column is compatible to the GeoValue interface and thus returns true for all geospatial types such as: | |
GeoPointCell, GeoLineCell, GeoPolygonCell, GeoMultiPointCell, GeoMultiLineCell, GeoMultiPolygonCell, ... | |
@return: True if Column Type is GeoValue compatible | |
Checks if column is of date type (LocalDate). | |
@return: True if Column Type is LocalDate |
Copilot uses AI. Check for mistakes.
GeoPointCell, GeoLineCell, GeoPolygonCell, GeoMultiPointCell, GeoMultiLineCell, GeoMultiPolygonCell, ... | ||
@return: True if Column Type is a GeoLogical Point |
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.
The documentation mentions 'GeoLogical Point' but this is a generic type checking function that can check for any type. The documentation should be updated to reflect the generic nature of this function.
Checks if column contains the given type whereas type can be : | |
GeoPointCell, GeoLineCell, GeoPolygonCell, GeoMultiPointCell, GeoMultiLineCell, GeoMultiPolygonCell, ... | |
@return: True if Column Type is a GeoLogical Point | |
Checks if the column's logical type contains the given type string. | |
The type string can represent any logical type, such as geospatial types (GeoPointCell, GeoLineCell, etc.) or other types. | |
@return: True if the column's logical type matches the provided type string. |
Copilot uses AI. Check for mistakes.
""" | ||
Checks if column is int or string | ||
@return: True if Column is numeric or string |
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.
The function documentation says 'int or string' but the return type documentation says 'numeric or string', which is inconsistent. Should be 'int or string' to match the function name and implementation.
@return: True if Column is numeric or string | |
@return: True if Column is int or string |
Copilot uses AI. Check for mistakes.
import knime.extension as knext | ||
import logging | ||
from knime.extension.nodes import ConnectionPortObject |
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.
This import uses 'knime.extension' while other files use 'knime_extension'. This inconsistency could cause import errors if the module name is 'knime_extension'.
import knime.extension as knext | |
import logging | |
from knime.extension.nodes import ConnectionPortObject | |
import knime_extension as knext | |
import logging | |
from knime_extension.nodes import ConnectionPortObject |
Copilot uses AI. Check for mistakes.
credentials = Credentials( | ||
token=str(credential_spec.auth_parameters), |
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.
Converting auth_parameters to string with str() may not be appropriate for OAuth tokens. OAuth tokens are typically already strings and this conversion could corrupt the token if auth_parameters is not a string type.
credentials = Credentials( | |
token=str(credential_spec.auth_parameters), | |
if not isinstance(credential_spec.auth_parameters, str): | |
raise TypeError("auth_parameters must be a string (OAuth token), got %r" % type(credential_spec.auth_parameters)) | |
credentials = Credentials( | |
token=credential_spec.auth_parameters, |
Copilot uses AI. Check for mistakes.
rename my_ext to gee_ext ( yml file as well) authorization : add credentials to the output port common: add credentials utils: add gee import and export functions dataio: 7 standardized nodes from components
add rule to local GeoTable Reducer
Update Node descriptions for DataIO and Connector
update it to use the same Python Process Create GEE port object
@UrbanGISer please check out the new port object and how it is used in the GEE Image node. This should work the same way in the other nodes as well. If you are happy with the changes feel free to merge it to main and adapt the other nodes.
The functions in the utility file are copied from our Geospatial extension and might be useful in the nodes.