changed the hardcoded format of spreadsheet API

[VinayCheripally]

Description

This PR fixes #703

Notes for Reviewers

Signed commits

• [ ] Yes, I signed my commits.

[gourav-k-shaw]

LGTM. @riyaa14 @lekaf974 you might have a feedback to offer

[leecalcote]

Has this been tested with the current Meshery Integrations sheet?

[lekaf974]

@VinayCheripally

Some questions come to me

1. We are making 3 times the same changes looks not a good practice IMO and not easy to maintain on long term
2. Since it is in meshkit and it will be reused by external client (mesheryctl and server) to ensure it is not breaking them, I suggest to create a test
3. Do we want to create a PR each time we will have to change it (if needed), we modify an hardcoded value by another harcoded one. What about to have a default value (which should be the current one) and add it override by an argument in the mehod ? This will allow to not break the current behavior and let the caller to override it on runtime if needed

[VinayCheripally]

@leecalcote I tested it and it is showing me that there zero models So ,I'm trying to resolve that.

[VinayCheripally]

@lekaf974 Yeah I think it makes sense to do what you are saying.I will work on them after I resolve this error with meshery Integration sheet.

[VinayCheripally]

@leecalcote It seems the current URL format which is https://docs.google.com/spreadsheets/d/{spreadsheet-id}/gviz/tq?tqx=out:csv&gid={gid} is meant for visualization and is not meant for downloading the sheet as a csv file. So I used the format https://docs.google.com/spreadsheets/d/{spreadsheet-id}/export?format=csv&gid={gid} which actually downloads the csv files.It works with both meshery Integrations sheet and also custom spreadsheets.

[VinayCheripally]

@lekaf974 I will add a test file to this.But I have a doubt ,using the default one with custom spreadsheets gives an error directly is that fine?

[lekaf974]

Can you share the error ?

[riyaa14]

Do we want to create a PR each time we will have to change it (if needed), we modify an hardcoded value by another harcoded one. What about to have a default value (which should be the current one) and add it override by an argument in the mehod ? This will allow to not break the current behavior and let the caller to override it on runtime if needed

I strongly agree with this point. Since there is no official documentation for this new URL format, it is good to have a default value and override it if needed.

[VinayCheripally]

Can you share the error ?

This is the error we are getting right now when using custom spreadsheets.

[leecalcote]

Is there are link pointing to where Google Sheets has changed their URL format?

[VinayCheripally]

Is there are link pointing to where Google Sheets has changed their URL format?

I did not find an official article for that change in format but I found this blog. https://youneedawiki.com/blog/posts/google-sheets-url-parameters.html

[riyaa14]

Is there are link pointing to where Google Sheets has changed their URL format?

I couldn't find any documentation too regarding the change of URL format or even on what URL formats are valid. @VinayCheripally suggested some solutions which again aren't mentioned officially but rather from different sources on the net.

[riyaa14]

But I have a doubt ,using the default one with custom spreadsheets gives an error directly is that fine?

@VinayCheripally actually this was the original problem. The URL format was working for Meshery Integrations sheet which I suppose is because it was published a long time back, but for recently published sheets, the URL wasn't working

and this is a problem as:

1. Users won't be able to use meshery registry commands for bulk model generation
2. E2E tests would need a new sheet too, we can't use the Meshery Integrations sheet here.

[riyaa14]

This is the error we are getting right now when using custom spreadsheets.

@VinayCheripally what do you mean by custom spreadsheets here?

Also, if you see the url format you shared here, how is it again calling the original URL format : /pub?output=csv&gid={sheet-id}? Shouldn't this call the updated URL - /export?format=csv&gid={sheet-id} as per your changes in this PR.

[lekaf974]

also could we validate there is no impact on other command before merging

[VinayCheripally]

This is the error we are getting right now when using custom spreadsheets.

@VinayCheripally what do you mean by custom spreadsheets here?

Also, if you see the url format you shared here, how is it again calling the original URL format : /pub?output=csv&gid={sheet-id}? Shouldn't this call the updated URL - /export?format=csv&gid={sheet-id} as per your changes in this PR.

I was showing this to @lekaf974 because he asked what kind of error was coming if we use the /pub?output=csv as default and override it by /export?format=csv if we use an argument. So,If we use this without argument in our tests for some test-spreadsheet it would give us an error and I asked @lekaf974 if that was fine or not.

[VinayCheripally]

This is the error we are getting right now when using custom spreadsheets.

@VinayCheripally what do you mean by custom spreadsheets here?

Also, if you see the url format you shared here, how is it again calling the original URL format : /pub?output=csv&gid={sheet-id}? Shouldn't this call the updated URL - /export?format=csv&gid={sheet-id} as per your changes in this PR.

By custom spreadsheets I meant something which user created by his own. Not the Meshery Integrations sheet.

[VinayCheripally]

https://developers.google.com/workspace/drive/api/reference/rest/v3/files/export This is the closest I could find about the URL format .there is a somewhat similar URL that is being explained here.

[VinayCheripally]

@lekaf974 @riyaa14 I have a doubt currently in meshkit for publishing the models to some path we are using the url https://docs.google.com/spreadsheets/d/[spreadsheet-id]/pub?output=csv&gid=[Sheet-ID] I believe this is used when we publish a spreadsheet to web which does not need credentials to access as we have already published it.So why are we using CRED variable in the mesheryctl command. Even for the url format https://docs.google.com/spreadsheets/d/{spreadsheet-id}/export?format=csv&gid=0 This url works only if the spreadsheet can be accessed by anyone with the link.It does not use any authorization. The only way which uses a .json file is the one with google sheets API which is not used directly to download the CSV file but to just read the file.So ,I'm confused why we are using the base64 format of .json file.

[lekaf974]

@lekaf974 @riyaa14 I have a doubt currently in meshkit for publishing the models to some path we are using the url https://docs.google.com/spreadsheets/d/[spreadsheet-id]/pub?output=csv&gid=[Sheet-ID] I believe this is used when we publish a spreadsheet to web which does not need credentials to access as we have already published it.So why are we using CRED variable in the mesheryctl command. Even for the url format https://docs.google.com/spreadsheets/d/{spreadsheet-id}/export?format=csv&gid=0 This url works only if the spreadsheet can be accessed by anyone with the link.It does not use any authorization. The only way which uses a .json file is the one with google sheets API which is not used directly to download the CSV file but to just read the file.So ,I'm confused why we are using the base64 format of .json file.

@VinayCheripally did not get involve in the command development but if you find some issues, create an issue with description then let's talk about it in slack and next dev meeting to get feedback.

I did not remind who worked on this implementation to give feedback, @leecalcote any idea ?

[lekaf974]

Pretty sure doing those e2e tests implementation will help detect issues that we were not aware due to small test coverage

[riyaa14]

@lekaf974 @riyaa14 I have a doubt currently in meshkit for publishing the models to some path we are using the url https://docs.google.com/spreadsheets/d/[spreadsheet-id]/pub?output=csv&gid=[Sheet-ID] I believe this is used when we publish a spreadsheet to web which does not need credentials to access as we have already published it.So why are we using CRED variable in the mesheryctl command. Even for the url format https://docs.google.com/spreadsheets/d/{spreadsheet-id}/export?format=csv&gid=0 This url works only if the spreadsheet can be accessed by anyone with the link.It does not use any authorization. The only way which uses a .json file is the one with google sheets API which is not used directly to download the CSV file but to just read the file.So ,I'm confused why we are using the base64 format of .json file.

@VinayCheripally spreadsheet-credentials (base64 encoded) are required for

1. Getting the Sheet-IDs
2. In registry generate command, we update the components sheet and without credentials, we won't be able to edit the sheet

These are two areas where I found the use of CRED, might be more

[VinayCheripally]

https://developers.google.com/workspace/drive/api/reference/rest/v3/files/export This is the closest I could find about the URL format .there is a somewhat similar URL that is being explained here.

@riyaa14 Does this look similar?It has the same use case.can you look at it once?

[VinayCheripally]

@VinayCheripally

Some questions come to me

1. We are making 3 times the same changes looks not a good practice IMO and not easy to maintain on long term

2. Since it is in meshkit and it will be reused by external client (mesheryctl and server) to ensure it is not breaking them, I suggest to create a test

3. Do we want to create a PR each time we will have to change it (if needed), we modify an hardcoded value by another harcoded one. What about to have a default value (which should be the current one) and add it override by an argument in the mehod ? This will allow to not break the current behavior and let the caller to override it on runtime if needed

I added a function in utils.go and used it at three places and by testing do you mean for only the function which I just changed or for all the functions in that model.go?

[VinayCheripally]

Renaming proposal also how it will modify the behavior on the runtime when mesheryctl will trigger one of those method ?

So whenever the defaultURI returns an error from the downloadFile function it uses the overrideURL and calls the same function again and checks for errors and just returns if it works as intended or else returns the error.

[lekaf974]

Renaming proposal also how it will modify the behavior on the runtime when mesheryctl will trigger one of those method ?

So whenever the defaultURI returns an error from the downloadFile function it uses the overrideURL and calls the same function again and checks for errors and just returns if it works as intended or else returns the error.

Ok, did not realized it on first read, in this case here it is important to add a comment this behavior explaining why we do this, please.

[VinayCheripally]

Renaming proposal also how it will modify the behavior on the runtime when mesheryctl will trigger one of those method ?

So whenever the defaultURI returns an error from the downloadFile function it uses the overrideURL and calls the same function again and checks for errors and just returns if it works as intended or else returns the error.

Ok, did not realized it on first read, in this case here it is important to add a comment this behavior explaining why we do this, please.

got it ,I'll do that.

[VinayCheripally]

@lekaf974 I added the comments.

[lekaf974]

@riyaa14 last check ok to merge then