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

Upgrades to integrations-hcl to support multiple strategies (Nomad Pack) #216

Merged
merged 8 commits into from
May 8, 2023

Conversation

BrandonRomano
Copy link
Contributor

@BrandonRomano BrandonRomano commented May 4, 2023

🎟️ Asana Task


Description

Upgrades the integrations-hcl codebase to support an input strategy parameter which influences the strategy of consumption.

This allows for differing consumption interfaces, which is required given our nomad-pack-community-registry has a different configuration interface.

PR Checklist 🚀

  • Conduct thorough self-review.
  • Add or update tests as appropriate.
  • Write a useful description (above) to give reviewers appropriate context.
  • Identify (in the description above) and document (add Asana tasks on this board) any technical debt that you're aware of, but are not addressing as part of this PR.

@changeset-bot
Copy link

changeset-bot bot commented May 4, 2023

🦋 Changeset detected

Latest commit: 0045982

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hashicorp/integrations-hcl Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@BrandonRomano BrandonRomano added the release:canary Publish a canary release label May 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

📦 Canary Packages Published

Latest commit: 0045982

Published 1 packages

@hashicorp/[email protected]

npm install @hashicorp/integrations-hcl@canary

app: app.array().length(1),
pack: pack.array().length(1),
integration: integration.array().length(1),
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at https://github.com/hashicorp/nomad-pack-community-registry/blob/main/packs/chaotic_ngine/metadata.hcl

Are app and pack Nomad-owned stanzas? and integration Digital-owned? It might be useful to note that here.

Also, we might want to add the missing app and pack fields here, even though we may not necessarily need them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are app and pack Nomad-owned stanzas? and integration Digital-owned? It might be useful to note that here.

Yes this is the right understanding. I will note this!

Also, we might want to add the missing app and pack fields here, even though we may not necessarily need them.

I would be concerned that we would be over-validating. For example, if they have a field xyz which is currently required, I wouldn't want to specify it as such in our configuration if we don't need to use it. In the event that the Nomad team wants to change their specification, I wouldn't want to have to change anything on our end (unless it was fields that we were touching).

@thiskevinwang thiskevinwang self-requested a review May 5, 2023 17:04
Copy link
Contributor

@thiskevinwang thiskevinwang left a comment

Choose a reason for hiding this comment

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

Looks pretty good! The separation of "strategies" is a huge improvement, especially so for the upcoming support of nomad-packs.

Just left a few suggestions to leave some comments/breadcrumbs for ourselves!

@BrandonRomano BrandonRomano changed the title [WIP] Upgrades to integrations-hcl to support multiple strategies (Nomad Pack) Upgrades to integrations-hcl to support multiple strategies (Nomad Pack) May 8, 2023
@BrandonRomano BrandonRomano merged commit 4b6af73 into main May 8, 2023
@BrandonRomano BrandonRomano deleted the br.nomad-pack branch May 8, 2023 21:18
@hashibot-web hashibot-web mentioned this pull request May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:canary Publish a canary release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants