Skip to content

[backend-only] cdp validation#6828

Draft
feederbox826 wants to merge 2 commits intostashapp:developfrom
feederbox826:cdp-test
Draft

[backend-only] cdp validation#6828
feederbox826 wants to merge 2 commits intostashapp:developfrom
feederbox826:cdp-test

Conversation

@feederbox826
Copy link
Copy Markdown
Member

took more time then I would've liked but it's working. Unsure of how to proceed with UI as it's just a string input :/ or if it should be merged backend-only?

validateStashBoxCredentials(input: StashBoxInput!): StashBoxValidationResult!

# Validate remote CDP path
ValidateCDPPath: CDPValidationResult!
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it be good to accept an input parameter here so that users can test before saving? Currently you cant test and it just saves regardless of it it is correct or not.

Should also do camel case here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

debated doing that as well

Comment thread pkg/scraper/url.go
}
if !isCDPPathHTTP(globalConfig) {
// unable to test non-http CDP
return fmt.Errorf("Unable to test non-http CDP paths")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So, should this be an error? I'm not sure on this one but it seems to me that the path could be valid but just not tested. It would return valid:false then spit the error out to the user. It could be misleading because it's not technically an error just not tested.

Perhaps it should be info level or something unless it's actually invalid.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

using the current way to fetch it, it cant handle it. We would need to do an actual xpath scrape to validate PATHs.

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.

2 participants