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

Adds auto_environment_variable smart attribute #3301

Closed
wants to merge 8 commits into from

Conversation

HazelGrant
Copy link
Contributor

Partially fixes #3081 - Cannot edit auto_environment_variables yet

Comment on lines 73 to 74
def build_smart_attributes(form: [], attributes: {}, job_environment: {})
attrs = form.map do |form_item_id|
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the addition of job_environment here - it should work just like all the other smart attributes.

I think the structure of this is a bit off - like you're doing a lot of the work in this model when a lot of this parsing & updating logic should be embedded in the smart attribute itself.

I.e., it knows how to build itself and apply attributes to it.

So given a YAML like this:

form:
  - auto_environment_variable_FOO
attributes:
  auto_environment_variable_FOO:
    value: bar

That would call the factory method like

#  SmartAttributes::AttributeFactory.build(form_item_id, attrs) evaluates to 
SmartAttributes::AttributeFactory.build('auto_environment_variable_FOO', { value: 'bar' })

Then in the factory - we can do something similar to what auto_modules does - the factory parses out FOO and supplies it as an argument to the builder

if id.match?(AUTO_MODULES_REX)
hpc_mod = id.match(AUTO_MODULES_REX)[1]
id = 'auto_modules'
opts = opts.merge({'module' => hpc_mod})
end

@HazelGrant HazelGrant closed this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

new project manager to support environment variables.
3 participants