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

GetComputeStatus directCommand and GET /compute have different results for same jobId #776

Closed
paulo-ocean opened this issue Dec 4, 2024 · 4 comments
Assignees
Labels
Type: Bug Something isn't working

Comments

@paulo-ocean
Copy link
Contributor

paulo-ocean commented Dec 4, 2024

For the same jobId the routes have different results (and both make use of getComputeStatus command)
with directCommand
Screenshot from 2024-12-04 15-08-24
using /compute endpoint
Screenshot from 2024-12-04 15-05-37

@paulo-ocean paulo-ocean added the Type: Bug Something isn't working label Dec 4, 2024
@paulo-ocean paulo-ocean changed the title GetComputeStatus directCommand and GET /compute have different results GetComputeStatus directCommand and GET /compute have different results for same jobId Dec 4, 2024
@paulo-ocean paulo-ocean self-assigned this Dec 4, 2024
@paulo-ocean
Copy link
Contributor Author

paulo-ocean commented Dec 4, 2024

Actually, the issue is with param agreementId, the 0x prefix is getting stripped out, when using the legacy route., which eventually leads to a mismatch if this field is used on the query

UPDATE: This is actually happening on ocean SDK computeStatus call done from the CLI

let url = `?consumerAddress=${consumerAddress}`
    url += (agreementId && `&agreementId=${this.noZeroX(agreementId)}`) || ''
    url += (jobId && `&jobId=${jobId}`) || ''

(the noZeroX call )
Not really sure why this was done like this, but better not touch the SDK atm, for backward compatibility... and just add the prefix if missing on Ocean Node side

@alexcos20
Copy link
Member

alexcos20 commented Dec 4, 2024

Not really sure why this was done like this, but better not touch the SDK atm, for backward compatibility

It was causing issues on provider, we can remove this in SDK for release 4

@paulo-ocean
Copy link
Contributor Author

Not really sure why this was done like this, but better not touch the SDK atm, for backward compatibility

It was causing issues on provider, we can remove this in SDK for release 4

ok, thanks.. will create a PR on the SDK as well for that

@paulo-ocean
Copy link
Contributor Author

closed on #777

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants