Skip to content

Migrate to hatch#56

Merged
richfitz merged 17 commits intomasterfrom
use-hatch
Jun 3, 2025
Merged

Migrate to hatch#56
richfitz merged 17 commits intomasterfrom
use-hatch

Conversation

@richfitz
Copy link
Copy Markdown
Member

@richfitz richfitz commented May 1, 2025

Before touching the constellation, this updates this package to (minimally) use hatch. I've not set up anything like the linting tools etc because we won't use it much longer.

The commit 09349df cherry-picks (modulo a small merge issue) the change from #53, made by @david-mears-2

The other big change is explicitly pulling all used images; this is due to this change in constellation: https://github.com/reside-ic/constellation/pull/22/files - alternatively we could have passed pull_images = True to every call to .start().

Closes #53, but contains the core commit

Copy link
Copy Markdown
Contributor

@david-mears-2 david-mears-2 left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -0,0 +1,2 @@
__version__ = "1.1.0"
__name__ = "pyorderly"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems wrong

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

indeed that is really quite wrong

Comment on lines +30 to +32
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.config.py }}
uses: actions/setup-python@v4
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These two actions (checkout, setup-python) have later versions available but perhaps those are not relevant

Comment on lines +60 to +62
# This can be useful, but the false positive rate is
# annoyingly high.
fail_ci_if_error: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes me go :/ - I'd rather people consciously choose to merge a PR that has red crosses on, than not notice the red crosses.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The error is on upload of the coverage report, failure running the coverage will still fail the build

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right, this is absolutely broken atm, I've removed this step for now as this package is essentially deprecated anyway

david-mears-2 added a commit to mrc-ide/packit-deploy that referenced this pull request May 2, 2025
This is due to a change in constellation 1.2.3 - see vimc/orderly-web-deploy#56
for an analogous fix
@richfitz richfitz requested a review from david-mears-2 May 12, 2025 12:44
@david-mears-2
Copy link
Copy Markdown
Contributor

david-mears-2 commented May 15, 2025

I might have approved too hastily because trying to run this locally I got

requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http+docker://localhost/v1.47/containers/create?name=orderly-web-proxy

and

docker.errors.ImageNotFound: 404 Client Error for http+docker://localhost/v1.47/containers/create?name=orderly-web-proxy: Not Found ("No such image: vimc/orderly-web-proxy:master")

here's the full output: https://pastebin.com/MTTkX11K

I had run these:

hatch shell
orderly-web start config/basic

Plausibly to do with me not being added to the right docker team or something.

@richfitz
Copy link
Copy Markdown
Member Author

You probably just need to add --pull to the start command?

@david-mears-2 david-mears-2 self-requested a review May 15, 2025 14:09
@david-mears-2
Copy link
Copy Markdown
Contributor

Good point

orderly-web start config/basic --pull allows me to actually start up

(I have to again switch the ports away from the defaults, and add the port to the configured web.url)

david-mears-2 added a commit to mrc-ide/packit-deploy that referenced this pull request May 16, 2025
This is due to a change in constellation 1.2.3 - see vimc/orderly-web-deploy#56
for an analogous fix
@richfitz richfitz merged commit e406223 into master Jun 3, 2025
6 checks passed
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.

2 participants