https://outerbounds.com/ logo
Join Slack
Powered by
hey <@U01NGJ0GA0J>, wanted to follow up on this th...
# dev-metaflow
w
hey @User, wanted to follow up on this thread https://github.com/Netflix/metaflow/pull/976#discussion_r824332464
✅ 1
a
meow wave
@wide-diamond-57477 What do you think about the proposal of exposing the secret in the environment directly as the secret name?
That will simplify the integration quite a bit? Otherwise, we can rely on a map - but it isn't very straightforward to declare a map through CLI
w
I think your proposal is worth talking through because there are some tradeoffs to think about In general, I’m thinking about two use cases: • Storing secrets as plain text • Storing secrets as JSON If we just extract the key from a plaintext 
secret:$secret_name
, this should be straightforward. For the JSON-style secrets, the user would specify 
secret:$secret_name:$json_key::
. One proposal would be to concatenate $secret_name and $json_key to create the environment variable. My biggest concern though is what characters are allowed in a secret name — for example, I just created the secret name 
9/_+=.@-
a
Yeah - I was just looking through allowed characters for secret names
In this scenario, we can then do 
@batch(secrets={'secret_name':'secret_arn'})
w
Sounds good, is there an existing example in the codebase I can take a look at?
a
@conda
takes in a map as args - you can take a look at that.
Also, we would need to ensure that step-functions also behaves as expected
I am happy to stress test the PR once you have finalized the changes. Let me know!
w
Sure thing, I have a flow set up on my side that’s deployed as step functions so can ensure that looks good before I ping again 🙂 This looks to just modify the AWS Batch job definition so I’m not anticipating any issues with the SFN integration
a
Yep - it should be pretty much smooth sailing
w
You had a good intuition — there seems to be separate paths for handling 
--with batch
arguments for 
run
versus 
step-functions create
The former will always hit this type conversion and by the time 
Batch
is instantiated, 
secrets
is a dictionary: https://github.com/stevenhoelscher/metaflow/blob/inject-secrets-into-aws-batch/metaflow/plugins/aws/batch/batch_cli.py#L279-L305 The latter will never hit the type conversion and by the time 
Batch
is instantiated, 
secrets
is still a JSON-encoded string: https://github.com/stevenhoelscher/metaflow/blob/inject-secrets-into-aws-batch/metaflow/plugins/aws/step_functions/step_functions.py#L654-L679 I’m hesitant to have additional logic to handle this edge case, so looking for input at your leisure
Hey Savin, wanted to check in and see if you have any advice on how to proceed
a
Interesting. Can I get back to you on this in a few hours?
👍 1
That's interesting - ideally this line should take care of the conversion for you
Open in Slack
PreviousNext
Powered by