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

Update instapy-docker to remove xpath_compile.py dependency #86

Merged
merged 6 commits into from
Nov 20, 2020

Conversation

elulcao
Copy link
Contributor

@elulcao elulcao commented Nov 18, 2020

Hello all

In past days it was asked to update xpath_compile.py due to some changes in InstaPy (repo #5895 and #5899),
the change was requested because of the dependencies between InstaPy-Docker and InstaPy.

  1. It means, that if some one changes or modify InstaPy - xpath_compile.py then the same change needs to be propagated to InstaPy-Docker - xpath_compile.py because of the Dockerfile copy the file to the Docker image.

  2. On the other hand, if someone ( like I did :p ) proactively changes the Instapy-Docker xpath_compile.py with the latest changes it will cause the Docker image to fail since internally there are parts that still cannot consume the new xpaths.

  3. https://hub.docker.com/r/instapy/instapy/dockerfile needs to be updated to reflect the changes in the new Dockerfile, if this PR is approved.

We could say that it is a game of cat and mouse, where it is changed first in a repository and the failure of the second, or the second repository is modified and the failure of the first is expected.

I created this PR to remove the xpath_compile.py dependency, so the user can always run InstaPy-Dockerwithout the urgency of checking if the xpath_compile is the correct one. I ran several times InstaPy-Docker with Docker-Compose and Dockerfile and never hit the issue mentioned in #4877.

I believe that a WA over how to start the Docker image is better if the user hit the same issue for extra characters like (```) or different xpath values; I have the impression that docker users know how to get around this type of configuration.

ie:
they can just map the correct xpath_compile directly from console, this way the changes can be done on the fly.

-v $(pwd)/xpath_compile.py:/usr/local/lib/python3.7/site-packages/instapy/xpath_compile.py:ro 

There are also another changes in docker-compose directory, it were removed unused files, the docker_quickstart.py.example was rename to docker_quickstart.py to be the same patter like InstaPy (repo), the documentation has been updated with the changes, the DockerFile has been formatted to the Docker standards and also the sed part has been updated to change the code without the necessity of using a line number.

Basically with these changes the aim is to make the creation of the Docker InstaPy image transparent, using the latest version of InstaPy and reducing the pull requests in this repository; ie: #84

Problem: xpath_compile.py is outdated if InstaPy main repo is changed
Solution: Remove xpath_compile.py dependency
Note:

Signed-off-by: elulcao <elulcao@icloud.com>
Problem: xpath_compile.py is outdated if InstaPy main repo is changed
Solution: Remove xpath_compile.py dependency
Note:

Signed-off-by: elulcao <elulcao@icloud.com>
Problem:
Solution: Line number is not nedeed with sed command
Note:

Signed-off-by: elulcao <elulcao@icloud.com>
Removed unused Dockerfile for docker-compose
Problem: NA
Solution: NA
Note: NA

Signed-off-by: elulcao <elulcao@icloud.com>
Problem:
Solution:
Note:

Signed-off-by: elulcao <elulcao@icloud.com>
Problem:
Solution:
Note:

Signed-off-by: elulcao <elulcao@icloud.com>
@timgrossmann
Copy link
Contributor

@elulcao Really nice summaries and updates, thank you very much for this!

@timgrossmann timgrossmann merged commit 4e85c78 into InstaPy:master Nov 20, 2020
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