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

As a data seller, I would like to disable consume for certain assets during certain activities like maintenance or downtime #452

Closed
alexcos20 opened this issue Mar 23, 2021 · 16 comments · Fixed by #463
Assignees
Labels
Type: Enhancement New feature or request

Comments

@alexcos20
Copy link
Member

alexcos20 commented Mar 23, 2021

By default all published assets are shown in the catalog.

Let's check for an additional flag in the DDO

-isMaintenance

if exists and it's true, then consume is disabled and we need to show a banner. All other functions are not affected (lp, etc)

Of course, during EditMetadata, publisher can update this field by checking/unchecking a checkbox

@alexcos20 alexcos20 added the Type: Enhancement New feature or request label Mar 23, 2021
@kianyee
Copy link
Contributor

kianyee commented Mar 24, 2021

Can we can add isMaintenance attribute under Config class instead of DDO.

This value can configure in env vars.

We can get the value of isMaintenance by calling getConfig function.

This can eliminate publisher to update DDO one by one.

@alexcos20
Copy link
Member Author

Under config, it will mean that your entire market is down, and not some assets only.

@srikanthkaja31 - can you detail more?

@srikanthkaja31
Copy link

We should have it at data asset level for owners to show/hide assets based on availability of data endpoints.

For marketplace outage, this is something we can deal on the UI side.

@kianyee
Copy link
Contributor

kianyee commented Mar 24, 2021

Got it, let's do it in asset level.

@alexcos20 , are we going check provider connectivity to disable asset consume when it's not available under this issue?

@alexcos20
Copy link
Member Author

That's a nice feature, let's open a issue for that

@kremalicious
Copy link
Contributor

kremalicious commented Mar 24, 2021

honestly, this all sounds like the wrong solution to a self-made devops problem:

  1. It is 2021 and there is no reason to have extended planned downtime for any web app or service, it is a solved devops problem in the age of cloud computing. Imagine Amazon or Rarible removing their buy buttons because they update some server in background for long time. Imagine Twitter removing the Tweet button because they update some server in the background. Imagine Uniswap disabling the Swap button. And so on. Every data publisher who runs their own provider needs to be made aware of providing proper availability of their service, and adjust their devops processes accordingly. There should never be extended periods of planned down time.
  2. If a market owner does something which will take functionality down, it is better to put up a generic maintenance page instead of surgically enabling/disabling features in the UI. Again, bad devops practices if there is a need for that. In any case, this is not something which should be in the core of the market.
  3. Ignoring all those thoughts, isMaintenance in itself is completely ambiguous and seems tailored for one specific use case, meaning it's not clear what "maintenance" means. Features should be named after what they do, not the situation they might get used in. E.g. in this case it seems like all that maintenance mode is supposed to do is disabling download/compute. Then let's be precise and, if we ignore that the original problem is a devops problem, this flag should be actually isConsumable. Which we already have but looks like this is exactly what the issue describes with maintenance mode.
  4. Giving data owners that option will encourage them to keep extended downtime of their provider. They should be nudged into providing proper availability, which is done by simply keeping the asset live at all times.

That being said, to me it seems the actual problem is that we want to signal to users before they buy that the underlying provider is down. In that case, we should simply ping the configured provider and show a small status under the buy button in case that ping fails.

@srikanthkaja31
Copy link

Kind of agree on above comments. can't think of any specific scenarios for show/hide assets.
The whole idea to enable consumers to know if the asset can be consumed or not based on provider and endpoint ping status. As long as we make it transparent to them before consumption this should be fine.
Generic maintenance page is fine in case of platform related issues.

Ques - If ping status is -ve, do we only show the status or disable the consume button?

@kremalicious
Copy link
Contributor

I think there are valid cases for hiding an asset, like #445 or #209, but what I want to prevent is to have many different "hide states" forcing us to deal with many conditions in our code.

For making provider status transparent there would be multiple ways:

  1. ping constantly when consume/compute component is mounted, and show warning in case of erroneous ping status. In this case we should then disable button since we have this pattern of disabling buttons already for other cases.
  2. ping as very first step after user hits Buy, in case of erroneous ping status drop out of flow and show an error.

To me 2. sounds most effective since not everybody who is on the asset details page wants to buy the asset.

@srikanthkaja31
Copy link

For hiding of assets use cases, we also have similar requirements. However we had it as a seperate issue as "option for data owners to delete assets which can no longer be consumed". If this is technically same as show/hide (as there is no such thing called delete) then hide feature makes sense.

@alexcos20
Copy link
Member Author

When detail page is displayed, we can call https://github.com/oceanprotocol/ocean.js/blob/main/src/provider/Provider.ts#L136 and make sure that asset's files are reachable.
If not, we can just disable the consume button (will disable buy as well in the current setup), but user can go to trade tab to buy DT if he wants (and asset has a pool setup, does not apply for FRE)

@alexcos20
Copy link
Member Author

My understanding was that a publisher may wish to temporarily disable an asset for a short time in future (ie: I will disable my asset for the next 2 days and after 2 days I will switch it back )

@srikanthkaja31

@srikanthkaja31
Copy link

srikanthkaja31 commented Mar 24, 2021

In summary there are multiple scenarios :

Data owner wants to put asset under maintenance for a short period of time either due to planned or unplanned events - hide data asset during this time.

Data owner wants to remove asset from listing as it is no longer valid - hide data asset permanently or other option to delete data asset.

Platform checks for provider and data endpoint connectivity and ping result is negative - it shows a message or disables consume option or combination of both?

@alexcos20 thx for reminding on the temporary hide option.

@kianyee
Copy link
Contributor

kianyee commented Mar 25, 2021

  1. Data owner wants to put asset under maintenance for a short period of time either due to planned or unplanned events - hide data asset during this time.

@kremalicious have his point that maintenance more on DevOps problem and should go for option 3

  1. Data owner wants to remove asset from listing as it is no longer valid - hide data asset permanently or other option to delete data asset.

Some similar issue already create like #445 or #209

  1. Platform checks for provider and data endpoint connectivity and ping result is negative - it shows a message or disables consume option or combination of both?

@kremalicious has proposal that to ping as very first step after user hits Buy, in case of erroneous ping status drop out of flow and show an error
At the same time, @alexcos20, we agree to open an issue for this

@srikanthkaja31 , could you please conclude which feature we need implement on this story?

@srikanthkaja31
Copy link

srikanthkaja31 commented Mar 25, 2021

@TSS-LauKY @krisliew As we discussed :

Pt 1 : Hiding of data asset is for data owners to make the asset unavailable for a short time for any process or other specific reason. This is intentional and not related to outage scenarios.

Pt 3 : This is more of provider and endpoint failure scenarios in which case the proposal on checking ping status as step 1 and show a message will work. @TSS-LauKY check if we want to create a new issue for this.

@alexcos20 @kremalicious For your advise.

@kianyee
Copy link
Contributor

kianyee commented Mar 26, 2021

By default all published assets are shown in the catalog.

Let's check for an additional flag in the DDO

-isMaintenance

if exists and it's true, then consume is disabled and we need to show a banner. All other functions are not affected (lp, etc)

Of course, during EditMetadata, publisher can update this field by checking/unchecking a checkbox

@alexcos20 , this story is for Pt 1, so we will go for approach you propose earlier

kianyee added a commit that referenced this issue Mar 29, 2021
@alexcos20 alexcos20 changed the title As a data seller, I would like to hide assets from the catalog during certain activities like maintenance or downtime As a data seller, I would like to disable consume for certain assets during certain activities like maintenance or downtime Apr 6, 2021
krisliew pushed a commit that referenced this issue Apr 15, 2021
@srikanthkaja31
Copy link

srikanthkaja31 commented Apr 18, 2021

Based on discussion with Ocean team, @alexcos20 to consolidate the below checks in one function for consumption which can be merged.

Allow All/Deny All : Data Publishers can explicitly allow or deny certain wallet address for consumption. This is done via "Edit Metadata" option. #513
Make Asset Non consumable : Data Publishers can set the asset to be non consumable during certain period. This is done via "Edit Metadata" option using "IsConsumable" field.
Ping Check : Marketplace checks for ping response from provider as 1st step during consumption. Transaction stops if there is no response. #459

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
8 participants