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

[Model WatchTower] Reduce exposure of Links logic #1820

Conversation

avishniakov
Copy link
Contributor

@avishniakov avishniakov commented Sep 18, 2023

Describe changes

I kept only the List functionality public for links. Creates and deletes will go only via SQL Store.
I kept one test running on the Rest Store - query empty list. Not sure, if I can somehow populate the list by switching to SQL for that purpose. If this is doable - guidance is appreciated.

Pre-requisites

Please ensure you have done the following:

  • I have read the CONTRIBUTING.md document.
  • If my change requires a change to docs, I have updated the documentation accordingly.
  • If I have added an integration, I have updated the integrations table and the corresponding website section.
  • I have added tests to cover my changes.
  • I have based my new branch on develop and the open PR is targeting develop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (add details above)

@avishniakov avishniakov changed the base branch from main to feature/OSS-2300-model-watch-tower-v0.1 September 18, 2023 14:12
@avishniakov
Copy link
Contributor Author

@fa9r @strickvl as requested 🙂

@avishniakov avishniakov added internal To filter out internal PRs and issues fix labels Sep 18, 2023
@github-actions github-actions bot added the enhancement New feature or request label Sep 18, 2023
@avishniakov
Copy link
Contributor Author

I have a question about Zen Stores:

  • I removed REST endpoints to create or delete links between Model Version and Artifacts to reduce exposure to very internal logic
  • I'm working on a class to be used in Annotation of step outputs
  • This class will have link_to_model method and inside I call Client().zen_store.creaate_.._link(...)

I imagine, that there is no warranty that this code will be executed towards SQL store, if used by Step Exit logic. Am I wrong and there is nothing to worry about? If I'm not wrong, how to ensure SQL execution?

Moreover: I start to get a headache once I imagine what should happen, if user is running on local stack connected to a remote server...

Help needed 🙂

@avishniakov
Copy link
Contributor Author

Closing as not feasible at the moment.

@avishniakov avishniakov deleted the feature/OSS-2418-remove-links-rest branch September 18, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fix internal To filter out internal PRs and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant