Skip to content

Conversation

lstein
Copy link
Collaborator

@lstein lstein commented Jul 20, 2023

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • [ X] Bug Fix
  • Optimization
  • Documentation Update

Have you discussed this change with the InvokeAI team?

  • [ X] Yes
  • No, because:

Description

This changes the "sync" route from a GET to POST method, in keeping with the Representational Existential(?) State Transfer (REST) protocol.

@lstein lstein marked this pull request as draft July 20, 2023 02:18
Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit pick change requested - but this is good, it allows the UI to automatically refresh all model lists.

For context, the UI is using a network fetching and caching library that supports tags for cache invalidation. What we can do is say "when this mutation occurs, invalidate the caches for all fetched model lists" and it just automagically happens

Co-authored-by: psychedelicious <4822129+psychedelicious@users.noreply.github.com>
@psychedelicious psychedelicious marked this pull request as ready for review July 20, 2023 05:08
@psychedelicious psychedelicious self-requested a review July 20, 2023 05:08
@blessedcoolant blessedcoolant requested a review from maryhipp as a code owner July 20, 2023 06:50
@blessedcoolant blessedcoolant changed the title change GET to POST method for model synchronization route feat: Add Sync Models Jul 20, 2023
@blessedcoolant blessedcoolant enabled auto-merge (squash) July 20, 2023 08:16
@blessedcoolant blessedcoolant disabled auto-merge July 20, 2023 08:16
@blessedcoolant blessedcoolant enabled auto-merge July 20, 2023 08:16
@blessedcoolant blessedcoolant merged commit 8d77c5c into main Jul 20, 2023
@blessedcoolant blessedcoolant deleted the fix/post-model-sync branch July 20, 2023 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants