Skip to content

Conversation

jjustin
Copy link
Contributor

@jjustin jjustin commented Aug 2, 2017

Script for maintaining repositories with RPM packages
Details on how to use are included in Readme.md

@jjustin jjustin requested a review from dominikznidar August 2, 2017 07:34
@jjustin jjustin force-pushed the rpm branch 2 times, most recently from 023397b to 708ac75 Compare August 7, 2017 09:49
Copy link
Contributor

@dominikznidar dominikznidar left a comment

Choose a reason for hiding this comment

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

Nicely done but needs some polishing.

rpm/README.md Outdated
Install package
```
su
yum install <package name>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just use sudo one liner.

rpm/README.md Outdated
Don't forget to increase the timeout of lambda function

If somebody tries to inject a malicious rpm file in your repo it will be automaticly added to repository. It is your job to make bucket secure enough for this not to happen.!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Take notes from the other MR into account.

rpm/README.md Outdated

## Notes

.rpm and repodata/* in repository directory are and should be publicly accessible
Copy link
Contributor

Choose a reason for hiding this comment

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

I think bullets (*) could improve readability of this section.

rpm/s3rpm.py Outdated
if os.environ['REPO_DIR'].endswith('/'):
os.environ['REPO_DIR'] = os.environ['REPO_DIR'][:-1]
if os.environ['REPO_DIR'].startswith('/'):
os.environ['REPO_DIR'] = os.environ['REPO_DIR'][1:]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think os.environ[X] should be treated as a readonly global variable even though python allows you to change it. Updates like this can introduce some weird and unexpected side effects.

repoDir  = os.environ['REPO_DIR'].strip('/') # is good enough :)

rpm/README.md Outdated
Don't forget to increase the timeout of lambda function

If somebody tries to inject a malicious rpm file in your repo it will be automaticly added to repository. It is your job to make bucket secure enough for this not to happen.!!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Instructions for running unit tests should be added.

rpm/README.md Outdated
```
git clone https://github.com/tactycal/lambdaRepos.git
cd lambdaRepos/rpm
pip3 install -t . -r requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

Dependencies are not stated here. It's always a good idea for the first second level title to be Requirements section.

rpm/Makefile Outdated
pip3 install -t . -r requirements.txt --upgrade

package:
zip code.zip $(ZIPPED)
Copy link
Contributor

Choose a reason for hiding this comment

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

test target would be useful.

@jjustin jjustin force-pushed the rpm branch 3 times, most recently from 1b695f6 to 5592f79 Compare August 22, 2017 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants