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

Add back resource_exists validator for uri field on File #52

Closed
evamaxfield opened this issue Mar 9, 2021 · 2 comments · Fixed by #101
Closed

Add back resource_exists validator for uri field on File #52

evamaxfield opened this issue Mar 9, 2021 · 2 comments · Fixed by #101
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers
Projects

Comments

@evamaxfield
Copy link
Member

We had to remove the usage of resources_exists from the uri field validator because we couldn't think of a method to pass the google credentials to the validator. At some point we may want to add this back but this shouldn't hold up #49 from being merged.

Actually after trying this, I think passing another param to the validator is a little tricky.

I can add kwargs to resource_exists, but I'm not sure if there's an easy way to pass in fs into that function inside uri = fields.TextField(required=True, validator=validators.resource_exists).

Because model fields in FireO are assigned like db_file.uri = "gs:https://stuff", I'm not sure we can pass in other params into the validators arg.

Would it make more sense to just validate separately outside of FireO's built-in field validation? Or is there an easier way to do this I'm missing?

Originally posted by @isaacna in #49 (comment)

@evamaxfield evamaxfield added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed and removed help wanted Extra attention is needed labels Mar 9, 2021
@isaacna
Copy link
Collaborator

isaacna commented Mar 12, 2021

Based on FireO's implementation, I don't think it's possible to pass in kwargs to a validator method passed in.

Two things we could do are:

  1. Make a PR to FireO to support extra validator args
  2. Handle validation on database models on our own. We could do something like create a base class with a validator field that takes in a callable, and a validator_args field. Then we can have each database model we have also inherit from that class.

I think 2 would be the easier option and give us more options to customize to our use cases in the future.

@sarahjliu sarahjliu added this to To do in v3.0 Jul 10, 2021
@evamaxfield evamaxfield moved this from Ready for Dev to Backlog in v3.0 Aug 11, 2021
@isaacna
Copy link
Collaborator

isaacna commented Aug 26, 2021

I added an option to pass kwargs to validator functions in this PR, so this should be doable now.

@evamaxfield evamaxfield moved this from Backlog to In progress in v3.0 Sep 8, 2021
v3.0 automation moved this from In progress to Done Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
No open projects
v3.0
Done
Development

Successfully merging a pull request may close this issue.

2 participants