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

fix zones constrains list parsing #1054

Merged
merged 1 commit into from
Jul 11, 2024
Merged

fix zones constrains list parsing #1054

merged 1 commit into from
Jul 11, 2024

Conversation

luissimas
Copy link
Contributor

Description

Closes #1050. This fixes the parsing of the zones constraint in bundles. The format in the Juju API is defined in: https://github.com/juju/juju/blob/3.6/core/constraints/constraints.go#L107.

QA Steps

The following python script can be used to verify both the bug in the current version as well as the fix implemented:

import asyncio
from juju.model import Model

bundle_file = "./bundle.yaml"

bundle = """
name: sample-bundle

series: jammy

machines:
  "0":
    constraints: zones=z-1

applications:
  postgresql:
    charm: postgresql
    channel: 14/stable
    num_units: 1
    to:
      - lxd:0
"""

async def main():
    with open(bundle_file, "w") as f:
        f.write(bundle)

    model = Model()
    await model.connect()
    await model.deploy(bundle_file)

asyncio.run(main())

All CI tests need to pass.

@jujubot
Copy link
Contributor

jujubot commented May 20, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

1 similar comment
@jujubot
Copy link
Contributor

jujubot commented May 20, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

Copy link
Contributor

@Aflynn50 Aflynn50 left a comment

Choose a reason for hiding this comment

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

This looks good, thanks for the fix!

@Aflynn50
Copy link
Contributor

/merge

@Aflynn50
Copy link
Contributor

Hey @luissimas, we can't currently merge your contribution as the commit is not signed and our repo has a rule disallowing merges of unverified commits.

Would you be able to follow this GitHub guide and force push a verified version of your commit?

@luissimas
Copy link
Contributor Author

Hey @luissimas, we can't currently merge your contribution as the commit is not signed and our repo has a rule disallowing merges of unverified commits.

Would you be able to follow this GitHub guide and force push a verified version of your commit?

Done. I've also rebased on the main branch.

@Aflynn50
Copy link
Contributor

/merge

@Aflynn50
Copy link
Contributor

/build

1 similar comment
@cderici
Copy link
Contributor

cderici commented Jun 27, 2024

/build

@Aflynn50
Copy link
Contributor

/merge

@cderici
Copy link
Contributor

cderici commented Jul 8, 2024

/build

@cderici
Copy link
Contributor

cderici commented Jul 11, 2024

/build

@cderici
Copy link
Contributor

cderici commented Jul 11, 2024

/merge

@jujubot jujubot merged commit c033058 into juju:main Jul 11, 2024
7 of 10 checks passed
jujubot added a commit that referenced this pull request Jul 11, 2024
#1071

## What's Changed
* fix parsing of storage constraints by @luissimas in #1053
* Add setuptools to tox.ini by @Aflynn50 in #1058
* fix(refresh): bug with revisions by @jack-w-shaw in #1067
* feat: conventional commits static analysis by @SimonRichardson in #1068
* fix(series): add noble support by @jack-w-shaw in #1063
* fix zones constrains list parsing by @luissimas in #1054
* fix(model): fix wrong instanciation of list-secrets facade by @gboutry in #1065
* fix(makefile): run .tox before lint in makefile target by @cderici in #1069
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect parsing of zones constraint
4 participants