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

[Housekeeping] Cleanup UrlBlob Usage #4632

Open
2 tasks done
pvditt opened this issue Dec 21, 2023 · 2 comments
Open
2 tasks done

[Housekeeping] Cleanup UrlBlob Usage #4632

pvditt opened this issue Dec 21, 2023 · 2 comments
Assignees
Labels
housekeeping Issues that help maintain flyte and keep it tech-debt free

Comments

@pvditt
Copy link
Contributor

pvditt commented Dec 21, 2023

Describe the issue

Remove usages of deprecated UrlBlob. UrlBlob is not utilized in propellor nor console but is returned in FlyteAdmin endpoints such as GetExecutionData.

What if we do not do this?

Just code cleanliness - potentially some confusion on how errors are getting bubbled up to clients.

Related component(s)

Flyteidl, Flyteconsole, Flytepropellor, Flyteadmin

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@pvditt pvditt added the housekeeping Issues that help maintain flyte and keep it tech-debt free label Dec 21, 2023
Copy link

welcome bot commented Dec 21, 2023

Thank you for opening your first issue here! 🛠

@pvditt pvditt self-assigned this Dec 21, 2023
Copy link

dosubot bot commented Dec 21, 2023

🤖

Hi @pvditt! I'm Dosu, a friendly bot here to assist you while we're waiting for our human maintainers. I can help you troubleshoot issues, answer your questions, and guide you on your contributor journey. So, feel free to ask away!

Based on the information provided, it seems feasible to remove the deprecated UrlBlob usage from FlyteAdmin endpoints such as GetExecutionData. However, it's crucial to ensure that it's not being used elsewhere in the codebase before proceeding with the removal.

To remove it, you can delete the AdminUrlBlob struct from the model_admin_url_blob.go file:

/*
 * flyteidl/service/admin.proto
 *
 * No description provided (generated by Swagger Codegen https://github.com/swagger-api/swagger-codegen)
 *
 * API version: version not set
 * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git)
 */

package flyteadmin

// Represents a string url and associated metadata used throughout the platform.
// type AdminUrlBlob struct {
// 	// Actual url value.
// 	Url string `json:"url,omitempty"`
// 	// Represents the size of the file accessible at the above url.
// 	Bytes string `json:"bytes,omitempty"`
// }

Then, remove all references to AdminUrlBlob in your codebase. If you're using an IDE, it should be able to help you find all usages of this struct.

Please note that this change could potentially break your code if there are any hidden dependencies on AdminUrlBlob. It's recommended to run your test suite after making this change to ensure everything still works as expected.

Regarding the specific reason why the UrlBlob was deprecated in the Flyte codebase, I wasn't able to find an answer within the repository. Similarly, I couldn't find where else in the Flyte codebase the deprecated UrlBlob is being used.

If you need further assistance or have more questions, feel free to ask.

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Issues that help maintain flyte and keep it tech-debt free
Projects
None yet
Development

No branches or pull requests

1 participant