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

endpoint update: /validate to /$validate #48

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

rpassas
Copy link
Contributor

@rpassas rpassas commented Jun 8, 2022

No description provided.

@rpassas rpassas requested a review from radamson June 8, 2022 20:32
@radamson
Copy link
Collaborator

radamson commented Jun 8, 2022

According to the validate operation definition the URL should be of the form URL: [base]/[Resource]/$validate and the operation cannot be run at the system level according to the spec. The most basic solution here could be a wildcard, which would work well enough for us because inferno-core and any other client could call with a valid Resource name and get a result. I'd start with this.

Also, I didn't mention this before, but I think we should add this new route alongside the existing route to ease our upgrade client upgrade path (i.e. we could merge this without needing a complementary inferno-core upgrade). Otherwise if we merge this and push a new dockerhub image and deploy inferno-core would break because it's not using the new route.


Side Note:
If we can get that working then we might be able to do something fancier where we get a list of children (maybe with Resource#listChildren?).

@ms-k1ngk0ng ms-k1ngk0ng self-requested a review June 13, 2022 16:39
Copy link
Contributor

@ms-k1ngk0ng ms-k1ngk0ng left a comment

Choose a reason for hiding this comment

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

Can you adjust the output of the tests? Currently it prints to STANDARD_OUT, while the other tests in the repo return a value of PASSED, SKIPPED, or FAILED. Let's get it to match the rest of the repo. It's worth looking at the Github actions failures messages to see what I'm talking about (the red X marks at the bottom of the PR conversation tab).

.vscode/settings.json Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
Copy link
Contributor

@ms-k1ngk0ng ms-k1ngk0ng left a comment

Choose a reason for hiding this comment

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

Can you rebase against main?

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.

4 participants