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

Dev pydev guide #149

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Dev pydev guide #149

merged 5 commits into from
Aug 7, 2024

Conversation

Zicchio
Copy link
Contributor

@Zicchio Zicchio commented Jul 30, 2024

Pull request #143 added ease of support for developing within a docker environment, but did not further explain how to do so and assumed that the reader has the technical capabilities and experties to perform this operation.

This assumption, in general, might not be true and this pull request aims at providing documentation on how to fill this gap.

Suggestion on how to improve this block of documentation is well accepted.


## Step 0: Identify which Python dependency requires development

We will assume that the developer want to develop a modified version of the library [eudi-wallet-it-python](https://github.com/italia/eudi-wallet-it-python) which is a dependency of the container `satosa-saml2spid`.
Copy link
Member

@peppelinux peppelinux Aug 6, 2024

Choose a reason for hiding this comment

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

Suggested change
We will assume that the developer want to develop a modified version of the library [eudi-wallet-it-python](https://github.com/italia/eudi-wallet-it-python) which is a dependency of the container `satosa-saml2spid`.
We assume that the developer needs to develop a modified version of the library [eudi-wallet-it-python](https://github.com/italia/eudi-wallet-it-python) which is a dependency of the container `satosa-saml2spid`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

applied in af64492


We will assume that the developer want to develop a modified version of the library [eudi-wallet-it-python](https://github.com/italia/eudi-wallet-it-python) which is a dependency of the container `satosa-saml2spid`.
A local copy of the library is required.
We will assume that the project eudi-wallet-it-python has been cloned in the folder `/home/username/my/development/folder/eudi-wallet-it-python/pyeudiw`. The path prefix `/home/username/my/development/folder/` is an example and should be replaced here with the location of your own development package.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
We will assume that the project eudi-wallet-it-python has been cloned in the folder `/home/username/my/development/folder/eudi-wallet-it-python/pyeudiw`. The path prefix `/home/username/my/development/folder/` is an example and should be replaced here with the location of your own development package.
We assume that the project eudi-wallet-it-python has been cloned in the folder `/home/username/my/development/folder/eudi-wallet-it-python/pyeudiw`. The path prefix `/home/username/my/development/folder/` is an example and should be replaced here with the location of your own development package.

Copy link
Contributor Author

@Zicchio Zicchio Aug 6, 2024

Choose a reason for hiding this comment

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

applied in af64492


## Step 4 (Optional): Install further dependencies in the container

If your version of the library containes further dependency, or if you want to install development only dependency such as, say [pdbpp](https://github.com/pdbpp/pdbpp), you can create a new image that contains the required dependency.
Copy link
Member

@peppelinux peppelinux Aug 6, 2024

Choose a reason for hiding this comment

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

Suggested change
If your version of the library containes further dependency, or if you want to install development only dependency such as, say [pdbpp](https://github.com/pdbpp/pdbpp), you can create a new image that contains the required dependency.
If your version of the library contains further dependencies, or if you want to install development only dependencies such as, say [pdbpp](https://github.com/pdbpp/pdbpp), you can create a new image that contains the required dependency or execute a terminal (such as a `bash`) within the container and install it manually, therefore commit the changes to the docker container, as shown in the next section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that docker container commit command actually creates a new image (see the docs) hence the proposed addition is technically redundant.

Since this might not be a well known detail of the command, I am happy to include the proposed addition.

4. Freeze the changes with the command `docker container commit satosa-saml2spid`.
5. Stop and then restart the container.

At the end of the procedure, you will have created a new updated image with the required dependency.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
At the end of the procedure, you will have created a new updated image with the required dependency.
At the end of the procedure, you will find the required dependency as part of your container.

@peppelinux
Copy link
Member

I suggest to link this pydev documentation in the main README, within the section dedicated to the developers

@peppelinux peppelinux requested a review from MdreW August 6, 2024 12:36
@Zicchio
Copy link
Contributor Author

Zicchio commented Aug 6, 2024

I suggest to link this pydev documentation in the main README, within the section dedicated to the developers

Done in b181b6e

@peppelinux peppelinux merged commit 8272442 into italia:master Aug 7, 2024
2 of 3 checks passed
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.

3 participants