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

Fixes for observed installation issues. #40

Merged
merged 7 commits into from
Oct 20, 2022

Conversation

davidjwbbc
Copy link
Contributor

Closes #39.

This PR fixes an issue with the creation of the OpenAPI bindings during some distribution or installation processes.
This also updates the documentation for the project to include new files, renamed files and to clarify the user privileges needed to run this with its default configuration.

Intercepts all standard build backend functions and runs checks to ensure that the OpenAPI bindings have been built. This fixes issues where rt_5gms_as.openapi_5g was not being included in some distributions or installs.
Updates the developer documentation to include new files and to change the names of renamed files in this project. Include information in the main README to make it clear that the application requires root privileges to run when used with the default configuration.
@davidjwbbc davidjwbbc added bug Something isn't working documentation Improvements or additions to documentation labels Oct 19, 2022
@davidjwbbc davidjwbbc added this to the MWC23 milestone Oct 19, 2022
@davidjwbbc davidjwbbc self-assigned this Oct 19, 2022
@davidjwbbc davidjwbbc linked an issue Oct 19, 2022 that may be closed by this pull request
docs/README.md Outdated
- generate_openapi - Will generate the OpenAPI python modules if not already present.
- docs/ - Development documentation and examples.
- generate_5gms_as_openapi - Will generate the OpenAPI python modules if not already present.
- docs/ - Development documentation and examples.
- README.md - This document.
- chc.json - An example ContentHostingConfiguration JSON file which provisions a pull-ingest AS for "Big Buck Bunny".
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't chc.json be removed now that the example is available from rt-common-shared/5gms?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a file inside docs/ which is not on the list? example-application-server.conf
the json has been moved to external/rt-commom-shared/..., isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

These are the two things I've fixed directly:
#39 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely

Copy link
Contributor

@rjb1000 rjb1000 left a comment

Choose a reason for hiding this comment

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

Thanks, @davidjwbbc.

Looks good apart from the above comment about removing chc.json from docs/README.md.

Let's see what @dsilhavy and @jordijoangimenez think.

@dsilhavy
Copy link
Contributor

dsilhavy commented Oct 19, 2022

Changes looks good to me, one observation: I am still getting a No module named 'rt_5gms_as.openapi_5g' on Mac. As pointed out in #39 Linux works for me though, so it might also be a problem on my machine. We would need someone with a Mac device to try as well.

jordijoangimenez and others added 2 commits October 19, 2022 13:39
../docs/rt-common-shared/5gms/examples/ContentHostingConfiguration_Big-Buck-Bunny_pull-ingest.json
is under ../external/
@davidjwbbc
Copy link
Contributor Author

I wonder if there's a problem not being reported in the generator script. You could try running build_scripts/generate_5gms_as_openapi to see the error messages.

This removes the description for chc.json, which has now moved to rt-common-shared and adds a description for the example application configuration file.
@rjb1000
Copy link
Contributor

rjb1000 commented Oct 19, 2022

Blocked awaiting approval from @dsilhavy and @jordijoangimenez.

Has @davidjwbbc addressed your feedback satisfactorily? If so, please hit the Approve button.

@jordijoangimenez
Copy link
Contributor

I believe this looks good. Daniel, if you are happy please go ahead.

@davidjwbbc davidjwbbc merged commit 6738267 into 5G-MAG:RC_v1.0.0 Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

MVP#1 Application Server installation issues/observations
4 participants