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.

Sign up for GitHub

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

Jump to bottom

Update dino-park-fence to Actix 4, Tokio 1 #79

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Update dino-park-fence to Actix 4, Tokio 1 #79

wants to merge 5 commits into from

Conversation

floatingatoll
Copy link
Contributor

@floatingatoll floatingatoll commented Jun 22, 2022 •
edited
Loading

This PR brings the dino-park-gate package into alignment so that we're not using a mix of 0.8 and 0.9 branches across our code.

Cargo.lock will need to be regenerated.

@floatingatoll floatingatoll changed the title Fix dino-park-* dependencies in all dino-park-* for consistency Update dino-park-gate dependency for consistency Jun 22, 2022
@floatingatoll
Copy link
Contributor Author

@bkochendorfer At your leisure, a test/deploy cycle on this (after regenerating Cargo.lock) would put -fence (0.8.2) onto the same dependency as -guard, -packs (0.9.1).

@bkochendorfer bkochendorfer requested a review from a team June 23, 2022 15:41
smarnach
smarnach requested changes Jun 24, 2022
View reviewed changes
Copy link

@smarnach smarnach left a comment

Choose a reason for hiding this comment

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

I just tried building this, and it looks like the new version doesn't build due to some version conflict.

@bkochendorfer
Copy link
Member

@smarnach can you make sure you have the latest? I just pushed up some fixes. This was more complicated than just bumping a version, it required updating actix.

@smarnach
Copy link

Actually, the latest version does build – I didn't have the commits you added a few mintues ago yet.

bkochendorfer
bkochendorfer reviewed Jun 24, 2022
View reviewed changes
src/graphql_api/root.rs
Err(e) => Err(e.into()),
// if there was an error and self_query is none
// enter nested match on enum error type CisClientError
Err(e) if self_query => match e {
Copy link
Member

Choose a reason for hiding this comment

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

downcast no longer existed on e and we are returning a CisClientError instead of a ProfileError. ProfileError is a variant of the CisClientError enum and ProfileDoesNotExist is a variant of the ProfileError enum. This seems to be functionally equivalent of what was here before.

@smarnach
Copy link

I can't build from this branch anymore:

$ cargo build
    Updating git repository `https://github.com/mozilla-iam/dino-park-gate`
error: failed to get `dino_park_gate` as a dependency of package `dino-park-fence v0.2.0 (/home/sven/moz/dino-park-fence)`

Caused by:
  failed to load source for dependency `dino_park_gate`

Caused by:
  Unable to update https://github.com/mozilla-iam/dino-park-gate?branch=0.9.1#ae8a7bda

Caused by:
  object not found - no match for id (ae8a7bda26c7618a7d8222c170c5204b923aab31); class=Odb (9); code=NotFound (-3)

I'm not sure what went wrong there, and unfortunately I don't have time to look into it before going on vacation.

@bkochendorfer
Copy link
Member

No problem @smarnach have a good vacation

@bkochendorfer
Copy link
Member

@smarnach I had the same issue, that version of dino-park-gate vanished so I updated to a newer one.

@floatingatoll
Copy link
Contributor Author

IAM-975 is working to stabilize the dependency versions without the version bumps that were necessary up above; I'll revisit this PR once that's landed to capture the version patching work.

@floatingatoll floatingatoll marked this pull request as draft October 27, 2022 07:23
@floatingatoll
Copy link
Contributor Author

@bkochendorfer Thanks for all the actix/tokio upgrade work here! I've rebased your work onto current so that it isn't lost, and will update this PR title to reflect that this incorporates your Actix/Tokio upgrade efforts. Much appreciated.

@floatingatoll floatingatoll changed the title Update dino-park-gate dependency for consistency Update dino-park-fence to Actix 4, Tokio 1 Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkochendorfer bkochendorfer bkochendorfer left review comments

@smarnach smarnach smarnach requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@bkochendorfer bkochendorfer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@floatingatoll @bkochendorfer @smarnach