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

development instructions are super confusing #113

Closed
6 tasks done
kremalicious opened this issue Jan 19, 2021 · 11 comments
Closed
6 tasks done

development instructions are super confusing #113

kremalicious opened this issue Jan 19, 2021 · 11 comments
Assignees
Labels
Type: Bug Something isn't working Type: Documentation Improvements or additions to documentation

Comments

@kremalicious
Copy link
Contributor

kremalicious commented Jan 19, 2021

Problem

The current instructions provide a terrible onboarding experience for both (a) devs just using ocean.py and (b) devs working to improve ocean.py code itself (in developers.md).

Use case: I'm an external developer and want to do a tiny code change and want to test my change locally. developers.md tells me to clone 3 repos and in the end no tests can run locally.

For testing flow, compare with this for inspiration https://github.com/oceanprotocol/ocean.js#-testing.

This needs:

For developers just using ocean.py (via import):

  • Get them to successfully go through "simple" and "marketplace" quickstarts in a streamlined fashion as possible. It's ok to use barge here. (How: use barge for both, and move the other stuff until after.) [Done in https://github.com/oceanprotocol/ocean.py/commit/a246fe4ee5515bb63d457660df9e71554161678b]
  • And then provide them with ways to understand how to do all the things towards their own app's production usage. That is: work with networks & services that aren't just local, e.g. to use rinkeby & provider/aquarius for rinkeby. And, use test OCEAN. And, understand how config params are set. [Already done via the "learn more" section]

For developers working to improve the ocean.py code itself:

  • for developers of ocean.py, figure out most minimal instructions to get tests running locally
  • The current development instructions provide 2 paths. Make it 1.
  • 1 simple way of running tests locally against locally running components. Please choose barge and not manual cloning/copying
  • same behavior between instructions and Travis (tox on Travis, pytest locally, why the difference?)

Related: #181: 'example' folder doesn't do much; either delete or flesh out

@kremalicious kremalicious added Type: Bug Something isn't working Type: Documentation Improvements or additions to documentation labels Jan 19, 2021
@kremalicious
Copy link
Contributor Author

kremalicious commented Jan 19, 2021

@alexcos20 @ssallam let's start by first figuring out the most basic way to run the tests locally. What else is required to make this succeed?

# 1st terminal
git clone https://github.com/oceanprotocol/barge
cd barge
./start_ocean.sh --no-dashboard

# 2nd terminal
tox

@mariacarmina what do you get when you run this?

@trentmc
Copy link
Member

trentmc commented Jan 19, 2021

I'm glad to see this as an issue 👍👍

@mariacarmina
Copy link
Member

@alexcos20 @ssallam let's start by first figuring out the most basic way to run the tests locally. What else is required to make this succeed?

# 1st terminal
git clone https://github.com/oceanprotocol/barge
cd barge
./start_ocean.sh --no-dashboard

# 2nd terminal
tox

@mariacarmina what do you get when you run this?

So, I want tot test locally the tests from ocean.py. I updated the provider's version, then I run barge with the provider too, I wait a little and the I run in the second terminal (in ocean.py) tox and I received this error and some timeouts.
newBarge

@trentmc
Copy link
Member

trentmc commented Jan 19, 2021

https://github.com/oceanprotocol/ocean.py/blob/master/READMEs/developers.md

This issue, and its related README file, are related to developers looking to improve ocean.py, vs simply use it.

.. It tells me to clone 3 repos and in the end no tests can run locally.
.. The current development instructions provide 2 paths but both of them do not allow me to run the tests locally in the end.
.. [need] figure out most minimal instructions to get tests running locally
.. [need] simple way of running tests locally against locally running components. Please choose barge and not manual cloning/copying same behavior between instructions and Travis (tox on Travis, pytest locally, why the difference?)
.. [need] testing flow, compare with this for inspiration https://github.com/oceanprotocol/ocean.js#-testing

I believe these the relevant issues that this ticket should be addressing.

Next use case: I'm a developer building a Django app and want to publish an asset.

This is a good idea. But outside the scope of this issue, since it's for developers simply using ocean.py. Go ahead and create that issue if you feel it's worth it:)

there is generally enough space in the main readme and no need to have extra development instructions, we could structure it the same as ocean.js, so a dedicated Testing part and maybe all the other sections too

90%+ users of ocean.py are just that -- users. They're looking to improve ocean.py. The README is currently structured to help them onboard the fastest, and walk through each sub-step.

Whereas people who want to learn how to improve ocean.py (including Testing part) get only a link to the developers.md file. This makes sense in terms of emphasis.

we could structure it the same as ocean.js ... and maybe all the other sections too

Remember, ocean.js has sub-READMEs for quickstarts, not in the main README. Same as ocean.py. They're what we agreed upon at the beginning of V3 work, and to me, still make lots of sense because they help a smooth onboarding.

The ocean.py READMEs they evolved based on user feedback and pain, most recently via #85 and #89. For example helping the developer find test OCEAN, and help understand how config params work. So, putting everything into a single README would be a step backward. Let's avoid that, and focus on the problems that this issue raises, copied at the top of this comment, which are to improve onboarding for developers looking to improve ocean.py.

BTW should we make a ticket to improve ocean.js README to help developers get test OCEAN, understand config params, etc?

let's start by first figuring out the most basic way to run the tests locally
I like this as a direction. Perfect:)

@mariacarmina
Copy link
Member

@alexcos20 @ssallam let's start by first figuring out the most basic way to run the tests locally. What else is required to make this succeed?

# 1st terminal
git clone https://github.com/oceanprotocol/barge
cd barge
./start_ocean.sh --no-dashboard

# 2nd terminal
tox

@mariacarmina what do you get when you run this?

So, I want tot test locally the tests from ocean.py. I updated the provider's version, then I run barge with the provider too, I wait a little and the I run in the second terminal (in ocean.py) tox and I received this error and some timeouts.
newBarge

Problem solved. When you update the barge, you need to delete the containers first: docker container rm $(docker ps -a -q).

@trentmc
Copy link
Member

trentmc commented Feb 25, 2021

Note: @akshay-ap had made some great changes so far, so to help move briskly in master, I just merged them.
25c6aee

@trentmc
Copy link
Member

trentmc commented Feb 25, 2021

I just merged in da350f3, which streamlines the flow further, to be barge-only. And it has a couple tweaks to avoid barge gotchas.

@trentmc
Copy link
Member

trentmc commented Mar 2, 2021

Today I cleaned up the description, clarifying between (a) devs just using ocean.py and (b) devs working to improve ocean.py code itself (in developers.md).

Turns out that (b) is in much better shape now. After many iterations by @akshay-ap and me.

To handle (a), I reassigned the issue to myself. Now that we have a simple flow for developers, we can propagate things back to users. I plan to move the simple & marketplace quickstarts to first-thing in the README, both using local / barge. Users can then learn details about remote services after they get these quick wins on local.

@trentmc
Copy link
Member

trentmc commented Mar 2, 2021

WIP to handle (a) is in this branch.

trentmc added a commit that referenced this issue Mar 5, 2021
For #113, change quickstarts to successfully go through "simple" and "marketplace" quickstarts in a streamlined fashion as possible. It's ok to use barge here.
@trentmc
Copy link
Member

trentmc commented Mar 5, 2021

(a) is done via commit a246fe4

@trentmc
Copy link
Member

trentmc commented Mar 5, 2021

Note: (a) would be better yet with the new issue I just reported: #200. "For users of ocean-lib with ganache, getting fake OCEAN adds friction". Beyond scope of this ticket.

And we can do much better yet with readthedocs style documentation. MultiRepo#80. Beyond scope of this ticket.

@kremalicious are you satisfied for this ticket now?

@trentmc trentmc closed this as completed Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants