oceanprotocol / ocean-cli

3 stars 4 forks source link

Make ocean-cli backwards compatible with as-is provider #66

Open mariacarmina opened 1 week ago

mariacarmina commented 1 week ago

Currently there is a check in our as-is provider regarding algorithm metadata:

def _build_and_validate_algo(self, algo_data):
        """Returns False if invalid, otherwise sets the validated_algo_dict attribute."""
        algorithm_did = algo_data.get("documentId")
        self.algo_service = None
        self.algo = None
        if algorithm_did and not algo_data.get("meta"):
            algorithm_tx_id = algo_data.get("transferTxId")
            algorithm_service_id = algo_data.get("serviceId")
            self.algo = get_asset_from_metadatastore(get_metadata_url(), algorithm_did)

Either we modify provider code, either we modify ocean-cli at meta filed for algorithm. I'll let this as discussion item. Here is my run logs:

ocean-provider-1         | Traceback (most recent call last):
ocean-provider-1         |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1523, in full_dispatch_request
ocean-provider-1         |     rv = self.dispatch_request()
ocean-provider-1         |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1509, in dispatch_request
ocean-provider-1         |     return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
ocean-provider-1         |   File "/ocean-provider/ocean_provider/routes/compute.py", line 70, in decorated_function
ocean-provider-1         |     return f(*args, **kws)
ocean-provider-1         |   File "/usr/local/lib/python3.8/site-packages/flask_sieve/validator.py", line 81, in wrapper
ocean-provider-1         |     return fn(*args, **kwargs)
ocean-provider-1         |   File "/ocean-provider/ocean_provider/routes/compute.py", line 457, in computeStart
ocean-provider-1         |     status = validator.validate()
ocean-provider-1         |   File "/ocean-provider/ocean_provider/validation/algo.py", line 40, in validate
ocean-provider-1         |     if not self.validate_input():
ocean-provider-1         |   File "/ocean-provider/ocean_provider/validation/algo.py", line 110, in validate_input
ocean-provider-1         |     status = self._build_and_validate_algo(algo_data)
ocean-provider-1         |   File "/ocean-provider/ocean_provider/validation/algo.py", line 245, in _build_and_validate_algo
ocean-provider-1         |     algorithm_dict = StageAlgoSerializer(
ocean-provider-1         |   File "/ocean-provider/ocean_provider/serializers.py", line 49, in serialize
ocean-provider-1         |     dict_template["container"] = self.algo_asset.metadata["algorithm"]["container"]
ocean-provider-1         | AttributeError: 'NoneType' object has no attribute 'metadata'
paulo-ocean commented 1 week ago

this is not a CLI issue its a bug on Provider It does NOT set self.algo, if not going into that IF self.algo = get_asset_from_metadatastore(get_metadata_url(), algorithm_did) Then it calls Serialize class with algo being None! and breaks here: self.algo_asset.metadata["algorithm"]["container"] (self.algo_asset is not defined) It should always call the get_asset_from_metadatastore or get it in some other way at least OR it should be fixed here:

class StageAlgoSerializer:
...
def serialize(self):
        algorithm_meta = self.algo_data.get("meta")
        algorithm_did = self.algo_data.get("documentId")
        algorithm_tx_id = self.algo_data.get("transferTxId")
....
dict_template["container"] = self.algo_asset.metadata["algorithm"]["container"]
...

If algo_asset is not defined, we should read dict_template["container"] from algorithm_meta instead

dict_template["container"] =algorithm_meta.get("container")

mariacarmina commented 1 week ago

I put it here, because I tested using it. @paulo-ocean I make the issue in order to be transparent also for @alexcos20. I set here two options depending how @alexcos20 wants to move on:

Either we modify provider code, either we modify ocean-cli at meta field for algorithm.

We know the problem, we know both root causes. Let's see if we are ok to deply a new provider or a new ocean-cli, in both cases we need a release.

paulo-ocean commented 1 week ago

one is the correct fix, other is a workaround to make it work, taking advantage of an existing bug, 2 wrongs don't make a right :-)

mariacarmina commented 1 week ago

Let @alexcos20 decide, he has the last word on which component we can do the change/release.

jamiehewitt15 commented 1 week ago

How long are we planning to support the existing provider for? It may not be worth the effort, we just tell everyone to use the node.