- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2
 
remove the concept of filesystem, we don't need it #114
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
          
     Merged
      
      
    Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    | 
           The change in CLI: diff --git a/packages/cli/src/AppComponent.js b/packages/cli/src/AppComponent.js
index 65dff24..1733c4c 100644
--- a/packages/cli/src/AppComponent.js
+++ b/packages/cli/src/AppComponent.js
@@ -1,25 +1,13 @@
-import { Page, createHttpFileSystem, createHyperparamFileSystem, getSource } from '@hyparam/components'
+import { Page, getHttpSource, getHyperparamSource } from '@hyparam/components'
 import React from 'react'
 
-const fileSystems = [
-  createHttpFileSystem(),
-  createHyperparamFileSystem({ endpoint: location.origin }),
-]
-
 export default function App() {
   const search = new URLSearchParams(location.search)
   const sourceId = search.get('key') ?? ''
   const row = search.get('row') === null ? undefined : Number(search.get('row'))
   const col = search.get('col') === null ? undefined : Number(search.get('col'))
 
-  let source = undefined
-  for (const fileSystem of fileSystems) {
-    const fsSource = getSource(sourceId, fileSystem)
-    if (fsSource) {
-      source = fsSource
-      break
-    }
-  }
+  const source = getHttpSource(sourceId) ?? getHyperparamSource(sourceId, { endpoint: location.origin })
 
   if (!source) {
     return React.createElement('div', { children: 'Could not load a data source. You have to pass a valid source in the url.' }) | 
    
              
                    platypii
  
              
              approved these changes
              
                  
                    Dec 5, 2024 
                  
              
              
            
            
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice simplification!
    
  severo 
      added a commit
        to hyparam/space
      that referenced
      this pull request
    
      Dec 6, 2024 
    
    
      
  
    
      
    
  
* setup GitHub Action * install python * github action env vars * gitignore * use vite to generate the app + update action * apply recommendations and fix errors * copy and adapt (ts) hyperparam-cli code only enable a Parquet URL, hardcoded * move assets to src/assets * fix css by renaming id * fix height down caret in sortable * add workers * update tsbuildinfo * enable Cell view * fix url * try this * specific code for hf space * fix urls * intercept click on link when staying in the app * use and set the 'key' query param * parse the HF urls * rename query param key to url * url->raw, prepare breadcrumb for file view * add action field + use in Cell * show content for all the routes + error, and fix %2F in branch name * fix query param * fix viewer and cell for 'blob' urls * add Folder view * fix text, markdown and image views * clean * change project url * add repository view, to select a branch * add home page * add home link + no view for 'base url' * namespace dataset sends to home * simplify code * add dependency to huggingface.js to remove code we will need it anyway later to enable authentication * add unstyled dataset search on home * add some style * test OAuth (requires space) * fix type * fix ts * comment out oauth for now * fix scroll on home * fix link * comment * upgrade hyparquet * remove unused code * upgrade hightable * upgrade hightable and style (row placeholder) * upgrade hightable and use cachedAsyncBuffer * Fix parquetQueryWorker for parallel requests * Optionally send chunks back from parquet worker * Parquet aware table provider * repository view: the main branch root path * test oauth * always try to get log in HF * refactor * use context to pass auth + return if auth not ready * remove traces * handle redirect + login/logout links * add debug * test * test * test * more logs * more debug * test * test * clean * fix token expiration + fix redirection + better UI for login/logout * hack css * handle gated datasets * fix private/gated for parquet / head requests * add fix for cell view: hyparam/hyperparam-cli@2fa7382 * add branch dropdown * update hightable * remove sidebar + change color + pass url to create breadcrumb * Revert "remove sidebar + change color + pass url to create breadcrumb" This reverts commit 68610c1. * move branch selector to the right + fix path parts * css tweak to have text and icon better aligned * add link to open the url * fix branch (refs/convert/parquet) in external link * handle middle mouse button on parquet view * refactoring to fix linter * move to folders hierarchy * update tests * fix file size * fix links in the HF space * fix Home link, to remove the ?url= param * upgrade hightable * Revamp the space with @hyparam/components. Temporarily without HF specifics * add CI * ignore coverage in eslint * upgrade packages, in particular @hyparam/components * add parseHuggingFaceUrl function * implement HF filesystem * upgrade components to 0.1.8 * enable auth - WARNING: relies on unreleased 0.1.10 see hyparam/hyperparam-cli#114 * fix lint * remove commented code * no need to stop the event propagation * remove commented code
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
It's embarrassing to refactor this part again, but it's really simpler this way. We need just one function per "filesystem," nothing more.