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

Configurable storage for new projects #3402

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

FrostyX
Copy link
Member

@FrostyX FrostyX commented Sep 5, 2024

See #2533

@FrostyX FrostyX added the pulp label Sep 5, 2024
@FrostyX FrostyX force-pushed the default-storage branch 2 times, most recently from 7d208bd to 406574f Compare September 5, 2024 10:36
Comment on lines 679 to 684
storage = wtforms.SelectField(
"Admin only - what storage should be set for new projects",
choices=[(x, x) for x in ["backend", "pulp"]],
validators=[wtforms.validators.Optional()],
)

Copy link
Member

Choose a reason for hiding this comment

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

Can we hide this option for normal users so they can't see it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I may have missed something but users shouldn't see it already. When creating/editing a project in the web UI, the option shouldn't be there for either admins or normal users. For API, it will probably have to be visible?

Also, I just realized that I forgot to add the field into the API schema and it works regardless. Is this expected @nikromen? I will add it though :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add it though :-)

Done

@@ -110,6 +110,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No
enabled together with this project repository.
:param list packit_forge_projects_allowed: List of forge projects that
will be allowed to build in the project via Packit
:param str storage: What storage should be set for new projects
Copy link
Member

Choose a reason for hiding this comment

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

Please add here the comment about admin only action too

Copy link
Member

Choose a reason for hiding this comment

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

Also I can see that in cli and frontend there are choices between pulp and BE. Can we somehow here check whether this is also storage enum or some typo nonsense?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please add here the comment about admin only action too

Good idea, I will mention it.

Can we somehow here check whether this is also storage enum or some typo nonsense?

That would be a precedent because so far we don't have validation for any fields in python-copr, only in CLI and then on frontend. Having validation there could be nice if the code could be shared between python-copr and copr-cli but I don't like the idea of duplicating the validation code on yet another place.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please add here the comment about admin only action too

Done

See fedora-copr#2533

This will be useful for beaker tests where we can now add basic tests
for every supported storage.
@praiskup praiskup merged commit 14faa50 into fedora-copr:main Sep 11, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants