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

Check types of RunParameter to catch errors and give user feedback #83

Open
LeonardHd opened this issue Mar 13, 2024 · 4 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@LeonardHd
Copy link
Collaborator

LeonardHd commented Mar 13, 2024

ADF only supports specific data types for what we call RunParameter and PipelineRunVariable.

We support python primitives that map to the data factory types.
For RunParameters this should be:

  • str -> String,
  • int -> Int,
  • float -> Float,
  • bool -> Bool
  • list -> Array
  • dict -> Object
  • str -> SecureString

For PipelineRunVariable these should be:

  • str -> String,
  • bool -> Boolean,
  • list-> Array,
  • int-> Integer,

Currently we do not validate any value on RunParameter and PipelineRunVariable.

@LeonardHd LeonardHd added enhancement New feature or request good first issue Good for newcomers labels Mar 13, 2024
@yehiaelbehery
Copy link
Contributor

Hi @LeonardHd, I can work on this.

@LeonardHd
Copy link
Collaborator Author

@yehiaelbehery Thanks for offering help. I will share some more details as this might help.
The example might be a bit too ambiguous, so I will collect some more details tomorrow 👍

@LeonardHd
Copy link
Collaborator Author

@arjendev I updated the issue. In fact, we do not validate anything at the moment in the framework.
I think simple guard clauses would be enough so only valid RunParameter and PipelineRunVariable could be instantiated. What do you think?

@yehiaelbehery let's see what Arjen's point of view here is, as he leads the API concepts.

@arjendev
Copy link
Collaborator

arjendev commented Apr 24, 2024

Sounds good!

@LeonardHd, perhaps we can also introduce the check to verify we pass in the right type of the output argument in activity.set_result and the output argument in state.add_activity_result. These should only accept dictionary and lists iirc. This was the place where you accidently pushed in a tuple and got an internal error, right?

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
None yet
Development

No branches or pull requests

3 participants