Skip to content
This repository was archived by the owner on Apr 17, 2023. It is now read-only.

Generate client omicron be46344 for 0.1.0-rc.39#111

Merged
karencfv merged 4 commits into
mainfrom
generate-client-omicron-be46344-for-0.1.0-rc.39
Jul 11, 2022
Merged

Generate client omicron be46344 for 0.1.0-rc.39#111
karencfv merged 4 commits into
mainfrom
generate-client-omicron-be46344-for-0.1.0-rc.39

Conversation

@karencfv

@karencfv karencfv commented Jul 8, 2022

Copy link
Copy Markdown
Collaborator

Updates generated client.

Due to oxidecomputer/omicron#1329 (comment), the spec will me modified to use the old operation IDs

@karencfv

karencfv commented Jul 8, 2022

Copy link
Copy Markdown
Collaborator Author

I am unsure why this GH action is failing in the "commit changes if any" step:

Your branch is up to date with 'origin/generate-client-omicron-be46344-for-0.1.0-rc.39'.
nothing to commit, working tree clean
Error: Process completed with exit code 1.

Even stranger, it seems this GH action runs randomly 😅 https://github.com/oxidecomputer/oxide.rs/actions/workflows/make-generate.yml

@augustuswm Might you have any idea as to why this is happening?

Comment thread Makefile
cargo build --bin generator

clean-spec:
$(RM) $(SPEC)

@augustuswm augustuswm Jul 8, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this an intended change? (Meant to link against the stray e that was added)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yep, for some reason sed is creating a spec.jsone file 🤷‍♀️ . Decided to leave it as this is a temporary workaround until the progenitor code is up and running (we won't need sed anymore with it)

@augustuswm

augustuswm commented Jul 8, 2022

Copy link
Copy Markdown

I haven't looked at this repo before, but this is my initial take:

The action looks set to run whenever a PR changes spec.json (or the workflow itself):

on:
  pull_request:
    paths:
      - spec.json
      - .github/workflows/make-generate.yml

The workflow is failing as it is expecting that a change to spec.json implies that running make generate will create changes. So when it runs git commit ... it is returning an exit code of 1 as there are no files to commit. Effectively the workflow wants to be the only one updating code in oxide/src.

That step of the workflow probably should be changed so that it can still succeed even if it does not generate any changes.

@karencfv

Copy link
Copy Markdown
Collaborator Author

Ahhhhhhhhhh, of course, gotcha. I brought back the spec.json file as we have a temporary workaround for operation IDs, so that must have triggered it. Thanks @augustuswm, much appreciated! 🙇‍♀️

@karencfv

karencfv commented Jul 11, 2022

Copy link
Copy Markdown
Collaborator Author

Removing the action all together. Like @augustuswm mentioned, it only gets triggered when the spec.json file changes. This means that it is a remnant from the old language client daisy chain that was already removed. The SDK will not be using the copy pasted spec.json file anymore, as it pulls directly from omicron. I am just leaving it here for now meanwhile we have to use the operation ID workaround oxidecomputer/omicron#1329 (comment)

@karencfv karencfv merged commit b63cc51 into main Jul 11, 2022
@karencfv karencfv deleted the generate-client-omicron-be46344-for-0.1.0-rc.39 branch July 11, 2022 00:38
@david-crespo

Copy link
Copy Markdown

Why do you need to check in spec.json for the workaround? Just to have something to reference to understand where the transformed names are coming from?

@karencfv

Copy link
Copy Markdown
Collaborator Author

Yes, and also to copy it to the CLI

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants