API end-point '/<env>/publish' created via FastAPI - request model

[lmilbaum]

/{env}/publish end point.

Run:
curl -X PUT http://localhost:8000/{env}/publish
Expected Result:
{"detail":"Created Publish Id"}

https://projects.engineering.redhat.com/browse/RHELDST-2774

Additional Information:
Changes to .pylintrc file were added due to an import-error. Following suggestion from the following KB:
https://stackoverflow.com/questions/1899436/pylint-unable-to-import-error-how-to-set-pythonpath

[negillett]

What's the intended scope of this PR?
What you have looks okay but I'm not seeing where the API would connect to or create publish objects in the database. Is that coming later?

[lmilbaum]

The scope of this PR is to act as the MVP of the feature. I am breaking the overall story implementation to baby steps/PRs, such that each PR is effectively re-viewable by the whole team.

[negillett]

This looks like a good base on which to build the API. So, +1 from me.

[rohanpm]

I am breaking the overall story implementation to baby steps/PRs, such that each PR is effectively re-viewable by the whole team.

I think this is too small, the way this is structured makes review less effective in my opinion rather than more effective, as it's unclear which parts of this PR (if any) aimed for completion.

Looking at the PR, what comes to mind includes:

• request model is incomplete
• response model is incomplete
• missing settings functionality for environments
• missing API docs
• missing implementation
• API for adding artifacts to a publish has appeared before the API for creating a publish, shouldn't it be the other way around?

This time I made inline comments on most of these, but I'm sure you were already aware of these points so I'm not sure if the review was just a waste of time. Maybe it would be better for a PR to try to complete at least one aspect of the work and point it out so that part could be reviewed?

If every single part of the PR falls into the category "it's not expected to be like this when it's finished" then I'm not sure what we're reviewing, it doesn't seem like there is much that can be done other than a rubber stamp +1 and a request to let us know when it's ready for proper review later.

[lmilbaum]

The PR should not be merged till #15 is. Some of the code changes in this PR will be removed as soon as #15 is merged and this PR is rebased.

[negillett]

The corresponding issue for this PR is about creating the publish objects in the database. The publish endpoint should only be responsible for creating the publish database object and that object's ID. So, I don't think it's in scope to publish_artifacts; that's encroaching on https://projects.engineering.redhat.com/browse/RHELDST-2160, I believe.

[lmilbaum]

The corresponding issue for this PR is about creating the publish objects in the database. The publish endpoint should only be responsible for creating the publish database object and that object's ID. So, I don't think it's in scope to publish_artifacts; that's encroaching on https://projects.engineering.redhat.com/browse/RHELDST-2160, I believe.

I have read again the issue and figured out my confusion. @nathanegillett thanks for that :-)