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

Move local-project dir inside sandcastle subfolder #2166

Conversation

majamassarini
Copy link
Member

@majamassarini majamassarini commented Aug 29, 2023

We need both upstream and dowstream repos in sandcastle shared folder.
For this reason I need to move the upstream repo from the root to a subfolder.

Fixes #1956

Merge after packit/packit#2054


RELEASE NOTES BEGIN
Packit now sets PACKIT_UPSTREAM_REPO and PACKIT_DOWNSTREAM_REPO environment variables for release syncing actions. The variables represent paths where the respective git repositories are cloned
RELEASE NOTES END

@softwarefactory-project-zuul
Copy link
Contributor

@majamassarini majamassarini force-pushed the fix/packit-service/1956 branch 2 times, most recently from 2200b49 to cca9cb9 Compare August 30, 2023 12:10
@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/508ae60da6814be09b5e68b07e5cf488

✔️ pre-commit SUCCESS in 1m 54s
✔️ packit-service-tests SUCCESS in 2m 16s
✔️ packit-service-tests-openshift SUCCESS in 14m 36s

packit_service/constants.py Outdated Show resolved Hide resolved
Comment on lines 263 to 264
SANDCASTLE_DG_REPO_DIR = "dg"
SANDCASTLE_LOCAL_PROJECT_DIR = "local-project"
Copy link
Member

Choose a reason for hiding this comment

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

I know the names don't matter much, but if local project working dir is in fact the upstream repo, why not use upstream and downstream/dist-git?

Copy link
Member Author

Choose a reason for hiding this comment

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

The LocalProjectMixin sets the suffix for this dir. The local project can be both an upstream or downstream project, it depends on the others mixins in the handler...
I can be sure only about the dg suffix, because it is used by the PackitAPIWithUpstreamMixin which is setting it.
The PackitAPIWithUpstreamMixin "depends" on the LocalProjectMixin so it is too late to specify the suffix for the upstream in PackitAPIWithUpstreamMixin.
And I have no other ideas...

I choose dg as a name because in the API is called like that, but I can change it in dist-git if you think it is better.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think dist-git is better, dg makes sense in the code as it is shorter, but I think we can afford to be a bit more explicit in directory names 🙂

Copy link
Member

@nforro nforro left a comment

Choose a reason for hiding this comment

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

LGTM, but I haven't tested it.

@nforro
Copy link
Member

nforro commented Aug 30, 2023

I'd change release notes to something like this:

Packit now sets PACKIT_UPSTREAM_REPO and PACKIT_DOWNSTREAM_REPO environment variables for release syncing actions. The variables represent paths where the respective git repositories are cloned.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/ecdb7d4426ef46c7a94c87ce44b737b2

✔️ pre-commit SUCCESS in 1m 44s
✔️ packit-service-tests SUCCESS in 1m 50s
✔️ packit-service-tests-openshift SUCCESS in 11m 59s

@majamassarini
Copy link
Member Author

LGTM, but I haven't tested it.

Me neither, because I can not test locally actions in sandcastle, since I have not openshift.
I can just see that the working dir for dg is under the shared sandcastle dir and I hope this will be enough.

@nforro
Copy link
Member

nforro commented Aug 31, 2023

Me neither, because I can not test locally actions in sandcastle, since I have not openshift.

It would be good if someone else had a look before merging, but I don't think this can break anything, so let's test it on stage.

We need both upstream and dowstream repos in sandcastle shared folder
for a sync_release.

Co-authored-by: Nikola Forró <nforro@redhat.com>
Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

thanks! let's test in staging

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/047328c2590c4e448922ce14497897c7

✔️ pre-commit SUCCESS in 1m 39s
✔️ packit-service-tests SUCCESS in 1m 50s
✔️ packit-service-tests-openshift SUCCESS in 13m 34s

softwarefactory-project-zuul bot added a commit to packit/packit that referenced this pull request Aug 31, 2023
Make dg repo accessible by an action

Fixes packit/packit-service#1956
Merge before packit/packit-service#2166

Reviewed-by: Nikola Forró
Reviewed-by: Maja Massarini
Reviewed-by: Laura Barcziová
@majamassarini majamassarini added the mergeit When set, zuul wil gate and merge the PR. label Aug 31, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/a55882f4305a40999f5854d545dc8a66

✔️ pre-commit SUCCESS in 1m 42s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit b7d6775 into packit:main Aug 31, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be able to access both upstream and downstream repos in actions
3 participants