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

chore: Add create_volume to swagger docs #161

Merged
merged 5 commits into from
Aug 29, 2023
Merged

Conversation

matheus23
Copy link
Member

  • Added a utoipa::path to the create_volume handler
  • Updated flake so postgres works in NixOS
  • Replaced ucan path dependencies with actual UCAN versions (@blaine this would still cause a runtime error, right? What's the github URL that I could depend on?)

@matheus23
Copy link
Member Author

Hm. We need a way to run kubo during tests.

@blaine
Copy link
Contributor

blaine commented Aug 23, 2023

  • Replaced ucan path dependencies with actual UCAN versions (@blaine this would still cause a runtime error, right? What's the github URL that I could depend on?)

Yeah, it's because the ucan:* "unscoped" bug in rs-ucan will result in proof chain validation failure. The "fix" I put in the vendored version was here: https://github.com/blaine/rs-ucan/tree/temporary-unscoped-fix (just a one line change), and I'm actually not sure the benefit of having a separate type for unscoped ucan: URIs so this may be the best overall fix (plus deleting related code)?

Anyhow, it's just on validating volume changes that things will fail until that issue gets resolved, or we can use a different resource (e.g. https://{domain}/api/account/{username}/*) instead of ucan:*

cd fission-server
diesel migration run --database-url $PGURL
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh god. The formatting messed this diff up quite badly 😅
I actually didn't change anything - it just lined up the "diesel migration run" with the "diesel database setup" lines from the two versions of the file.
These scripts are literally unchanged!

@matheus23
Copy link
Member Author

Ah - sorry for the huge diff in flake.nix. I actually have a nix formatter installed.

@walkah
Copy link
Member

walkah commented Aug 28, 2023

Hm. We need a way to run kubo during tests.

Might make more sense to just mock for CI?

Either way, can we get this merged so it's easier to build & run and work on a kubo testing strategy in a different PR?

@matheus23
Copy link
Member Author

Yeah, let's merge.

@matheus23 matheus23 merged commit 85fd8df into main Aug 29, 2023
2 of 6 checks passed
@matheus23 matheus23 deleted the matheus23/swagger-route branch August 29, 2023 09:32
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